From ad7e39075637b071b4d3a5812850166a3df68c49 Mon Sep 17 00:00:00 2001
From: Carlos Rodriguez <carlos@interchain.io>
Date: Tue, 16 May 2023 14:33:44 +0200
Subject: [PATCH] imp: do not store total escrow when amount is zero (#3585)

* do not store 0 escrow amout

* adapt success test

* Update modules/apps/transfer/keeper/keeper.go

Co-authored-by: Jim Fasarakis-Hilliard <d.f.hilliard@gmail.com>

* Update modules/apps/transfer/keeper/keeper.go

Co-authored-by: Damian Nolan <damiannolan@gmail.com>

* add comments

---------

Co-authored-by: Jim Fasarakis-Hilliard <d.f.hilliard@gmail.com>
Co-authored-by: Damian Nolan <damiannolan@gmail.com>
---
 modules/apps/transfer/keeper/keeper.go      | 13 +++++++++++--
 modules/apps/transfer/keeper/keeper_test.go | 20 ++++++++++++++++++--
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/modules/apps/transfer/keeper/keeper.go b/modules/apps/transfer/keeper/keeper.go
index d7032c2bfab..da2d2a6a05c 100644
--- a/modules/apps/transfer/keeper/keeper.go
+++ b/modules/apps/transfer/keeper/keeper.go
@@ -152,7 +152,7 @@ func (k Keeper) IterateDenomTraces(ctx sdk.Context, cb func(denomTrace types.Den
 func (k Keeper) GetTotalEscrowForDenom(ctx sdk.Context, denom string) sdk.Coin {
 	store := ctx.KVStore(k.storeKey)
 	bz := store.Get(types.TotalEscrowForDenomKey(denom))
-	if bz == nil {
+	if len(bz) == 0 {
 		return sdk.NewCoin(denom, sdk.ZeroInt())
 	}
 
@@ -163,14 +163,23 @@ func (k Keeper) GetTotalEscrowForDenom(ctx sdk.Context, denom string) sdk.Coin {
 }
 
 // SetTotalEscrowForDenom stores the total amount of source chain tokens that are in escrow.
+// Amount is stored in state if and only if it is not equal to zero. The function will panic
+// if the amount is negative.
 func (k Keeper) SetTotalEscrowForDenom(ctx sdk.Context, coin sdk.Coin) {
 	if coin.Amount.IsNegative() {
 		panic(fmt.Sprintf("amount cannot be negative: %s", coin.Amount))
 	}
 
 	store := ctx.KVStore(k.storeKey)
+	key := types.TotalEscrowForDenomKey(coin.Denom)
+
+	if coin.Amount.IsZero() {
+		store.Delete(key) // delete the key since Cosmos SDK x/bank module will prune any non-zero balances
+		return
+	}
+
 	bz := k.cdc.MustMarshal(&sdk.IntProto{Int: coin.Amount})
-	store.Set(types.TotalEscrowForDenomKey(coin.Denom), bz)
+	store.Set(key, bz)
 }
 
 // GetAllTotalEscrowed returns the escrow information for all the denominations.
diff --git a/modules/apps/transfer/keeper/keeper_test.go b/modules/apps/transfer/keeper/keeper_test.go
index c22489391cc..235fa9cf9f6 100644
--- a/modules/apps/transfer/keeper/keeper_test.go
+++ b/modules/apps/transfer/keeper/keeper_test.go
@@ -60,7 +60,7 @@ func (suite *KeeperTestSuite) TestSetGetTotalEscrowForDenom() {
 		expPass  bool
 	}{
 		{
-			"success: with 0 escrow amount",
+			"success: with non-zero escrow amount",
 			func() {},
 			true,
 		},
@@ -71,6 +71,13 @@ func (suite *KeeperTestSuite) TestSetGetTotalEscrowForDenom() {
 			},
 			true,
 		},
+		{
+			"success: escrow amount 0 is not stored",
+			func() {
+				expAmount = math.ZeroInt()
+			},
+			true,
+		},
 		{
 			"failure: setter panics with negative escrow amount",
 			func() {
@@ -85,7 +92,7 @@ func (suite *KeeperTestSuite) TestSetGetTotalEscrowForDenom() {
 
 		suite.Run(tc.name, func() {
 			suite.SetupTest() // reset
-			expAmount = math.ZeroInt()
+			expAmount = math.NewInt(100)
 			ctx := suite.chainA.GetContext()
 
 			tc.malleate()
@@ -94,6 +101,15 @@ func (suite *KeeperTestSuite) TestSetGetTotalEscrowForDenom() {
 				suite.chainA.GetSimApp().TransferKeeper.SetTotalEscrowForDenom(ctx, sdk.NewCoin(denom, expAmount))
 				total := suite.chainA.GetSimApp().TransferKeeper.GetTotalEscrowForDenom(ctx, denom)
 				suite.Require().Equal(expAmount, total.Amount)
+
+				storeKey := suite.chainA.GetSimApp().GetKey(types.ModuleName)
+				store := ctx.KVStore(storeKey)
+				key := types.TotalEscrowForDenomKey(denom)
+				if expAmount.IsZero() {
+					suite.Require().False(store.Has(key))
+				} else {
+					suite.Require().True(store.Has(key))
+				}
 			} else {
 				suite.Require().PanicsWithError("negative coin amount: -1", func() {
 					suite.chainA.GetSimApp().TransferKeeper.SetTotalEscrowForDenom(ctx, sdk.NewCoin(denom, expAmount))