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 <[email protected]> * Update modules/apps/transfer/keeper/keeper.go Co-authored-by: Damian Nolan <[email protected]> * add comments --------- Co-authored-by: Jim Fasarakis-Hilliard <[email protected]> Co-authored-by: Damian Nolan <[email protected]>
cosmos · May 16, 2023 · ad7e390 · ad7e390
1 parent 5b6e8b4
commit ad7e390
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 4 deletions.
diff --git 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
@@ -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))