refactor: make fee storage more efficient (#956)

* adding new proto types and codegen * refactoring ics29 fees for more efficient storage * updating tests * fixing typo in protodoc comments
cosmos · Feb 23, 2022 · b02d193 · b02d193
1 parent 4fb6d18
commit b02d193
Show file tree

Hide file tree

Showing 12 changed files with 724 additions and 144 deletions.
diff --git a/docs/ibc/proto-docs.md b/docs/ibc/proto-docs.md
@@ -32,6 +32,8 @@
     - [Fee](#ibc.applications.fee.v1.Fee)
     - [IdentifiedPacketFee](#ibc.applications.fee.v1.IdentifiedPacketFee)
     - [IdentifiedPacketFees](#ibc.applications.fee.v1.IdentifiedPacketFees)
+    - [PacketFee](#ibc.applications.fee.v1.PacketFee)
+    - [PacketFees](#ibc.applications.fee.v1.PacketFees)
 
 - [ibc/applications/fee/v1/genesis.proto](#ibc/applications/fee/v1/genesis.proto)
     - [FeeEnabledChannel](#ibc.applications.fee.v1.FeeEnabledChannel)
@@ -742,12 +744,46 @@ and an optional list of relayers that are permitted to receive the fee.
 <a name="ibc.applications.fee.v1.IdentifiedPacketFees"></a>
 
 ### IdentifiedPacketFees
-IdentifiedPacketFees contains a list of packet fees
+IdentifiedPacketFees contains a list of type PacketFee and associated PacketId
 
 
 | Field | Type | Label | Description |
 | ----- | ---- | ----- | ----------- |
-| `packet_fees` | [IdentifiedPacketFee](#ibc.applications.fee.v1.IdentifiedPacketFee) | repeated |  |
+| `packet_id` | [ibc.core.channel.v1.PacketId](#ibc.core.channel.v1.PacketId) |  |  |
+| `packet_fees` | [PacketFee](#ibc.applications.fee.v1.PacketFee) | repeated |  |
+
+
+
+
+
+
+<a name="ibc.applications.fee.v1.PacketFee"></a>
+
+### PacketFee
+PacketFee contains the relayer fee, refund address and an optional list of relayers that are permitted to receive the
+fee
+
+
+| Field | Type | Label | Description |
+| ----- | ---- | ----- | ----------- |
+| `fee` | [Fee](#ibc.applications.fee.v1.Fee) |  |  |
+| `refund_address` | [string](#string) |  |  |
+| `relayers` | [string](#string) | repeated |  |
+
+
+
+
+
+
+<a name="ibc.applications.fee.v1.PacketFees"></a>
+
+### PacketFees
+PacketFees contains a list of type PacketFee
+
+
+| Field | Type | Label | Description |
+| ----- | ---- | ----- | ----------- |
+| `packet_fees` | [PacketFee](#ibc.applications.fee.v1.PacketFee) | repeated |  |
 
 
 

diff --git a/modules/apps/29-fee/ibc_module.go b/modules/apps/29-fee/ibc_module.go
@@ -226,13 +226,13 @@ func (im IBCModule) OnAcknowledgementPacket(
 	}
 
 	packetID := channeltypes.NewPacketId(packet.SourceChannel, packet.SourcePort, packet.Sequence)
-	identifiedPacketFees, found := im.keeper.GetFeesInEscrow(ctx, packetID)
+	feesInEscrow, found := im.keeper.GetFeesInEscrow(ctx, packetID)
 	if !found {
 		// return underlying callback if no fee found for given packetID
 		return im.app.OnAcknowledgementPacket(ctx, packet, ack.Result, relayer)
 	}
 
-	im.keeper.DistributePacketFees(ctx, ack.ForwardRelayerAddress, relayer, identifiedPacketFees.PacketFees)
+	im.keeper.DistributePacketFees(ctx, ack.ForwardRelayerAddress, relayer, feesInEscrow.PacketFees)
 
 	// removes the fees from the store as fees are now paid
 	im.keeper.DeleteFeesInEscrow(ctx, packetID)
@@ -253,13 +253,13 @@ func (im IBCModule) OnTimeoutPacket(
 	}
 
 	packetID := channeltypes.NewPacketId(packet.SourceChannel, packet.SourcePort, packet.Sequence)
-	identifiedPacketFees, found := im.keeper.GetFeesInEscrow(ctx, packetID)
+	feesInEscrow, found := im.keeper.GetFeesInEscrow(ctx, packetID)
 	if !found {
 		// return underlying callback if fee not found for given packetID
 		return im.app.OnTimeoutPacket(ctx, packet, relayer)
 	}
 
-	im.keeper.DistributePacketFeesOnTimeout(ctx, relayer, identifiedPacketFees.PacketFees)
+	im.keeper.DistributePacketFeesOnTimeout(ctx, relayer, feesInEscrow.PacketFees)
 
 	// removes the fee from the store as fee is now paid
 	im.keeper.DeleteFeesInEscrow(ctx, packetID)

diff --git a/modules/apps/29-fee/ibc_module_test.go b/modules/apps/29-fee/ibc_module_test.go
@@ -307,8 +307,8 @@ func (suite *FeeTestSuite) TestOnChanCloseInit() {
 					1,
 				)
 				refundAcc := suite.chainA.SenderAccount.GetAddress()
-				identifiedFee := types.NewIdentifiedPacketFee(packetID, types.Fee{validCoins, validCoins2, validCoins3}, refundAcc.String(), []string{})
-				err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, identifiedFee)
+				packetFee := types.NewPacketFee(types.Fee{validCoins, validCoins2, validCoins3}, refundAcc.String(), []string{})
+				err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee)
 				suite.Require().NoError(err)
 			},
 			false,
@@ -322,8 +322,8 @@ func (suite *FeeTestSuite) TestOnChanCloseInit() {
 					Sequence:  1,
 				}
 				refundAcc := suite.chainA.SenderAccount.GetAddress()
-				identifiedFee := types.NewIdentifiedPacketFee(packetID, types.Fee{validCoins, validCoins2, validCoins3}, refundAcc.String(), []string{})
-				err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, identifiedFee)
+				packetFee := types.NewPacketFee(types.Fee{validCoins, validCoins2, validCoins3}, refundAcc.String(), []string{})
+				err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee)
 				suite.Require().NoError(err)
 
 				suite.chainA.GetSimApp().BankKeeper.SendCoinsFromModuleToAccount(suite.chainA.GetContext(), types.ModuleName, refundAcc, validCoins3)
@@ -385,8 +385,8 @@ func (suite *FeeTestSuite) TestOnChanCloseConfirm() {
 					Sequence:  1,
 				}
 				refundAcc := suite.chainA.SenderAccount.GetAddress()
-				identifiedFee := types.NewIdentifiedPacketFee(packetID, types.Fee{validCoins, validCoins2, validCoins3}, refundAcc.String(), []string{})
-				err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, identifiedFee)
+				packetFee := types.NewPacketFee(types.Fee{validCoins, validCoins2, validCoins3}, refundAcc.String(), []string{})
+				err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee)
 				suite.Require().NoError(err)
 			},
 			false,
@@ -400,8 +400,8 @@ func (suite *FeeTestSuite) TestOnChanCloseConfirm() {
 					Sequence:  1,
 				}
 				refundAcc := suite.chainA.SenderAccount.GetAddress()
-				identifiedFee := types.NewIdentifiedPacketFee(packetID, types.Fee{validCoins, validCoins2, validCoins3}, refundAcc.String(), []string{})
-				err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, identifiedFee)
+				packetFee := types.NewPacketFee(types.Fee{validCoins, validCoins2, validCoins3}, refundAcc.String(), []string{})
+				err := suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee)
 				suite.Require().NoError(err)
 
 				suite.chainA.GetSimApp().BankKeeper.SendCoinsFromModuleToAccount(suite.chainA.GetContext(), types.ModuleName, refundAcc, validCoins3)
@@ -544,7 +544,7 @@ func (suite *FeeTestSuite) TestOnRecvPacket() {
 func (suite *FeeTestSuite) TestOnAcknowledgementPacket() {
 	var (
 		ack                    []byte
-		identifiedFee          types.IdentifiedPacketFee
+		packetFee              types.PacketFee
 		originalBalance        sdk.Coins
 		expectedBalance        sdk.Coins
 		expectedRelayerBalance sdk.Coins
@@ -558,7 +558,7 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() {
 		{
 			"success",
 			func() {
-				expectedRelayerBalance = identifiedFee.Fee.RecvFee.Add(identifiedFee.Fee.AckFee[0])
+				expectedRelayerBalance = packetFee.Fee.RecvFee.Add(packetFee.Fee.AckFee[0])
 			},
 			true,
 		},
@@ -606,7 +606,7 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() {
 					ForwardRelayerAddress: blockedAddr.String(),
 				}.Acknowledgement()
 
-				expectedRelayerBalance = identifiedFee.Fee.AckFee
+				expectedRelayerBalance = packetFee.Fee.AckFee
 			},
 			true,
 		},
@@ -630,8 +630,7 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() {
 
 			// escrow the packet fee
 			packetID := channeltypes.NewPacketId(packet.GetSourceChannel(), packet.GetSourcePort(), packet.GetSequence())
-			identifiedFee = types.NewIdentifiedPacketFee(
-				packetID,
+			packetFee = types.NewPacketFee(
 				types.Fee{
 					RecvFee:    validCoins,
 					AckFee:     validCoins2,
@@ -640,7 +639,7 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() {
 				suite.chainA.SenderAccount.GetAddress().String(),
 				[]string{},
 			)
-			err = suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, identifiedFee)
+			err = suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee)
 			suite.Require().NoError(err)
 
 			relayerAddr := suite.chainB.SenderAccount.GetAddress()
@@ -656,7 +655,7 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() {
 			originalBalance = sdk.NewCoins(suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), ibctesting.TestCoin.Denom))
 
 			// default to success case
-			expectedBalance = originalBalance.Add(identifiedFee.Fee.TimeoutFee[0])
+			expectedBalance = originalBalance.Add(packetFee.Fee.TimeoutFee[0])
 
 			// malleate test case
 			tc.malleate()
@@ -687,7 +686,7 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() {
 func (suite *FeeTestSuite) TestOnTimeoutPacket() {
 	var (
 		relayerAddr     sdk.AccAddress
-		identifiedFee   types.IdentifiedPacketFee
+		packetFee       types.PacketFee
 		originalBalance sdk.Coins
 		expectedBalance sdk.Coins
 	)
@@ -727,8 +726,8 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() {
 				relayerAddr = suite.chainA.GetSimApp().AccountKeeper.GetModuleAccount(suite.chainA.GetContext(), transfertypes.ModuleName).GetAddress()
 
 				expectedBalance = originalBalance.
-					Add(identifiedFee.Fee.RecvFee[0]).
-					Add(identifiedFee.Fee.AckFee[0])
+					Add(packetFee.Fee.RecvFee[0]).
+					Add(packetFee.Fee.AckFee[0])
 			},
 			false,
 		},
@@ -753,8 +752,7 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() {
 			// must be explicitly changed
 			relayerAddr = suite.chainB.SenderAccount.GetAddress()
 
-			identifiedFee = types.NewIdentifiedPacketFee(
-				packetID,
+			packetFee = types.NewPacketFee(
 				types.Fee{
 					RecvFee:    validCoins,
 					AckFee:     validCoins2,
@@ -764,7 +762,7 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() {
 				[]string{},
 			)
 
-			err = suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, identifiedFee)
+			err = suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), packetID, packetFee)
 			suite.Require().NoError(err)
 
 			// log original sender balance
@@ -773,8 +771,8 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() {
 
 			// default to success case
 			expectedBalance = originalBalance.
-				Add(identifiedFee.Fee.RecvFee[0]).
-				Add(identifiedFee.Fee.AckFee[0])
+				Add(packetFee.Fee.RecvFee[0]).
+				Add(packetFee.Fee.AckFee[0])
 
 			// malleate test case
 			tc.malleate()
@@ -793,7 +791,7 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() {
 				found := suite.chainA.GetSimApp().IBCFeeKeeper.HasFeesInEscrow(suite.chainA.GetContext(), packetID)
 				suite.Require().False(found)
 
-				suite.Require().Equal(identifiedFee.Fee.TimeoutFee, relayerBalance)
+				suite.Require().Equal(packetFee.Fee.TimeoutFee, relayerBalance)
 			} else {
 				suite.Require().Empty(relayerBalance)
 			}

diff --git a/modules/apps/29-fee/keeper/escrow.go b/modules/apps/29-fee/keeper/escrow.go
@@ -11,13 +11,13 @@ import (
 )
 
 // EscrowPacketFee sends the packet fee to the 29-fee module account to hold in escrow
-func (k Keeper) EscrowPacketFee(ctx sdk.Context, packetID channeltypes.PacketId, identifiedFee types.IdentifiedPacketFee) error {
+func (k Keeper) EscrowPacketFee(ctx sdk.Context, packetID channeltypes.PacketId, packetFee types.PacketFee) error {
 	if !k.IsFeeEnabled(ctx, packetID.PortId, packetID.ChannelId) {
 		// users may not escrow fees on this channel. Must send packets without a fee message
 		return sdkerrors.Wrap(types.ErrFeeNotEnabled, "cannot escrow fee for packet")
 	}
 	// check if the refund account exists
-	refundAcc, err := sdk.AccAddressFromBech32(identifiedFee.RefundAddress)
+	refundAcc, err := sdk.AccAddressFromBech32(packetFee.RefundAddress)
 	if err != nil {
 		return err
 	}
@@ -27,26 +27,26 @@ func (k Keeper) EscrowPacketFee(ctx sdk.Context, packetID channeltypes.PacketId,
 		return sdkerrors.Wrapf(types.ErrRefundAccNotFound, "account with address: %s not found", refundAcc)
 	}
 
-	coins := identifiedFee.Fee.Total()
+	coins := packetFee.Fee.Total()
 	if err := k.bankKeeper.SendCoinsFromAccountToModule(ctx, refundAcc, types.ModuleName, coins); err != nil {
 		return err
 	}
 
-	packetFees := []types.IdentifiedPacketFee{identifiedFee}
+	fees := []types.PacketFee{packetFee}
 	if feesInEscrow, found := k.GetFeesInEscrow(ctx, packetID); found {
-		packetFees = append(packetFees, feesInEscrow.PacketFees...)
+		fees = append(fees, feesInEscrow.PacketFees...)
 	}
 
-	identifiedFees := types.NewIdentifiedPacketFees(packetFees)
-	k.SetFeesInEscrow(ctx, packetID, identifiedFees)
+	packetFees := types.NewPacketFees(fees)
+	k.SetFeesInEscrow(ctx, packetID, packetFees)
 
-	EmitIncentivizedPacket(ctx, identifiedFee)
+	EmitIncentivizedPacket(ctx, packetID, packetFee)
 
 	return nil
 }
 
 // DistributePacketFees pays the acknowledgement fee & receive fee for a given packetId while refunding the timeout fee to the refund account associated with the Fee.
-func (k Keeper) DistributePacketFees(ctx sdk.Context, forwardRelayer string, reverseRelayer sdk.AccAddress, feesInEscrow []types.IdentifiedPacketFee) {
+func (k Keeper) DistributePacketFees(ctx sdk.Context, forwardRelayer string, reverseRelayer sdk.AccAddress, feesInEscrow []types.PacketFee) {
 	forwardAddr, _ := sdk.AccAddressFromBech32(forwardRelayer)
 
 	for _, packetFee := range feesInEscrow {
@@ -73,7 +73,7 @@ func (k Keeper) DistributePacketFees(ctx sdk.Context, forwardRelayer string, rev
 }
 
 // DistributePacketsFeesTimeout pays the timeout fee for a given packetId while refunding the acknowledgement fee & receive fee to the refund account associated with the Fee
-func (k Keeper) DistributePacketFeesOnTimeout(ctx sdk.Context, timeoutRelayer sdk.AccAddress, feesInEscrow []types.IdentifiedPacketFee) {
+func (k Keeper) DistributePacketFeesOnTimeout(ctx sdk.Context, timeoutRelayer sdk.AccAddress, feesInEscrow []types.PacketFee) {
 	for _, feeInEscrow := range feesInEscrow {
 		// check if refundAcc address works
 		refundAddr, err := sdk.AccAddressFromBech32(feeInEscrow.RefundAddress)
@@ -113,8 +113,8 @@ func (k Keeper) RefundFeesOnChannel(ctx sdk.Context, portID, channelID string) e
 
 	var refundErr error
 
-	k.IterateIdentifiedChannelFeesInEscrow(ctx, portID, channelID, func(identifiedFees types.IdentifiedPacketFees) (stop bool) {
-		for _, identifiedFee := range identifiedFees.PacketFees {
+	k.IteratePacketFeesInEscrow(ctx, portID, channelID, func(packetFees types.PacketFees) (stop bool) {
+		for _, identifiedFee := range packetFees.PacketFees {
 			refundAccAddr, err := sdk.AccAddressFromBech32(identifiedFee.RefundAddress)
 			if err != nil {
 				refundErr = err