chore: update ics29 genesis state to support multiple packet fees (#957)

* adding new proto types and codegen * refactoring ics29 fees for more efficient storage * updating tests * updating genesis protos to use IdentifiedPacketFees * updating init/export genesis state functionality and tests
cosmos · Feb 23, 2022 · 15fa37b · 15fa37b
1 parent b02d193
commit 15fa37b
Show file tree

Hide file tree

Showing 12 changed files with 136 additions and 90 deletions.
diff --git a/docs/ibc/proto-docs.md b/docs/ibc/proto-docs.md
@@ -846,7 +846,7 @@ GenesisState defines the fee middleware genesis state
 
 | Field | Type | Label | Description |
 | ----- | ---- | ----- | ----------- |
-| `identified_fees` | [IdentifiedPacketFee](#ibc.applications.fee.v1.IdentifiedPacketFee) | repeated |  |
+| `identified_fees` | [IdentifiedPacketFees](#ibc.applications.fee.v1.IdentifiedPacketFees) | repeated |  |
 | `fee_enabled_channels` | [FeeEnabledChannel](#ibc.applications.fee.v1.FeeEnabledChannel) | repeated |  |
 | `registered_relayers` | [RegisteredRelayerAddress](#ibc.applications.fee.v1.RegisteredRelayerAddress) | repeated |  |
 | `forward_relayers` | [ForwardRelayerAddress](#ibc.applications.fee.v1.ForwardRelayerAddress) | repeated |  |

diff --git a/modules/apps/29-fee/keeper/genesis.go b/modules/apps/29-fee/keeper/genesis.go
@@ -8,8 +8,8 @@ import (
 
 // InitGenesis initializes the fee middleware application state from a provided genesis state
 func (k Keeper) InitGenesis(ctx sdk.Context, state types.GenesisState) {
-	for _, fee := range state.IdentifiedFees {
-		k.SetFeeInEscrow(ctx, fee)
+	for _, identifiedFees := range state.IdentifiedFees {
+		k.SetFeesInEscrow(ctx, identifiedFees.PacketId, types.NewPacketFees(identifiedFees.PacketFees))
 	}
 
 	for _, relayer := range state.RegisteredRelayers {

diff --git a/modules/apps/29-fee/keeper/genesis_test.go b/modules/apps/29-fee/keeper/genesis_test.go
@@ -9,11 +9,7 @@ import (
 func (suite *KeeperTestSuite) TestInitGenesis() {
 	// build PacketId & Fee
 	refundAcc := suite.chainA.SenderAccount.GetAddress()
-	packetId := channeltypes.NewPacketId(
-		ibctesting.FirstChannelID,
-		ibctesting.MockFeePort,
-		uint64(1),
-	)
+	packetID := channeltypes.NewPacketId(ibctesting.FirstChannelID, ibctesting.MockFeePort, 1)
 	fee := types.Fee{
 		RecvFee:    defaultReceiveFee,
 		AckFee:     defaultAckFee,
@@ -25,12 +21,16 @@ func (suite *KeeperTestSuite) TestInitGenesis() {
 	counterparty := suite.chainB.SenderAccount.GetAddress().String()
 
 	genesisState := types.GenesisState{
-		IdentifiedFees: []types.IdentifiedPacketFee{
+		IdentifiedFees: []types.IdentifiedPacketFees{
 			{
-				PacketId:      packetId,
-				Fee:           fee,
-				RefundAddress: refundAcc.String(),
-				Relayers:      nil,
+				PacketId: packetID,
+				PacketFees: []types.PacketFee{
+					{
+						Fee:           fee,
+						RefundAddress: refundAcc.String(),
+						Relayers:      nil,
+					},
+				},
 			},
 		},
 		FeeEnabledChannels: []types.FeeEnabledChannel{
@@ -51,9 +51,9 @@ func (suite *KeeperTestSuite) TestInitGenesis() {
 	suite.chainA.GetSimApp().IBCFeeKeeper.InitGenesis(suite.chainA.GetContext(), genesisState)
 
 	// check fee
-	identifiedFee, found := suite.chainA.GetSimApp().IBCFeeKeeper.GetFeeInEscrow(suite.chainA.GetContext(), packetId)
+	feesInEscrow, found := suite.chainA.GetSimApp().IBCFeeKeeper.GetFeesInEscrow(suite.chainA.GetContext(), packetID)
 	suite.Require().True(found)
-	suite.Require().Equal(genesisState.IdentifiedFees[0], identifiedFee)
+	suite.Require().Equal(genesisState.IdentifiedFees[0].PacketFees, feesInEscrow.PacketFees)
 
 	// check fee is enabled
 	isEnabled := suite.chainA.GetSimApp().IBCFeeKeeper.IsFeeEnabled(suite.chainA.GetContext(), ibctesting.MockFeePort, ibctesting.FirstChannelID)
@@ -78,8 +78,8 @@ func (suite *KeeperTestSuite) TestExportGenesis() {
 		TimeoutFee: defaultTimeoutFee,
 	}
 
-	identifiedPacketFee := types.NewIdentifiedPacketFee(packetID, fee, refundAcc.String(), []string{})
-	suite.chainA.GetSimApp().IBCFeeKeeper.SetFeeInEscrow(suite.chainA.GetContext(), identifiedPacketFee)
+	packetFee := types.NewPacketFee(fee, refundAcc.String(), []string{})
+	suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, types.NewPacketFees([]types.PacketFee{packetFee}))
 
 	// relayer addresses
 	sender := suite.chainA.SenderAccount.GetAddress().String()
@@ -99,9 +99,9 @@ func (suite *KeeperTestSuite) TestExportGenesis() {
 
 	// check fee
 	suite.Require().Equal(packetID, genesisState.IdentifiedFees[0].PacketId)
-	suite.Require().Equal(fee, genesisState.IdentifiedFees[0].Fee)
-	suite.Require().Equal(refundAcc.String(), genesisState.IdentifiedFees[0].RefundAddress)
-	suite.Require().Equal([]string(nil), genesisState.IdentifiedFees[0].Relayers)
+	suite.Require().Equal(fee, genesisState.IdentifiedFees[0].PacketFees[0].Fee)
+	suite.Require().Equal(refundAcc.String(), genesisState.IdentifiedFees[0].PacketFees[0].RefundAddress)
+	suite.Require().Equal([]string(nil), genesisState.IdentifiedFees[0].PacketFees[0].Relayers)
 
 	// check registered relayer addresses
 	suite.Require().Equal(sender, genesisState.RegisteredRelayers[0].Address)

diff --git a/modules/apps/29-fee/keeper/keeper.go b/modules/apps/29-fee/keeper/keeper.go
@@ -328,15 +328,29 @@ func (k Keeper) HasFeeInEscrow(ctx sdk.Context, packetId channeltypes.PacketId)
 }
 
 // GetAllIdentifiedPacketFees returns a list of all IdentifiedPacketFees that are stored in state
-func (k Keeper) GetAllIdentifiedPacketFees(ctx sdk.Context) []types.IdentifiedPacketFee {
+func (k Keeper) GetAllIdentifiedPacketFees(ctx sdk.Context) []types.IdentifiedPacketFees {
 	store := ctx.KVStore(k.storeKey)
-	iterator := sdk.KVStorePrefixIterator(store, []byte(types.FeeInEscrowPrefix))
+	iterator := sdk.KVStorePrefixIterator(store, []byte(types.FeesInEscrowPrefix))
 	defer iterator.Close()
 
-	var identifiedFees []types.IdentifiedPacketFee
+	var identifiedFees []types.IdentifiedPacketFees
 	for ; iterator.Valid(); iterator.Next() {
-		fee := k.MustUnmarshalFee(iterator.Value())
-		identifiedFees = append(identifiedFees, fee)
+		keySplit := strings.Split(string(iterator.Key()), "/")
+
+		feesInEscrow := k.MustUnmarshalFees(iterator.Value())
+
+		channelID, portID := keySplit[2], keySplit[1]
+		seq, err := strconv.ParseUint(keySplit[3], 10, 64)
+		if err != nil {
+			panic(err)
+		}
+
+		identifiedFee := types.IdentifiedPacketFees{
+			PacketId:   channeltypes.NewPacketId(channelID, portID, seq),
+			PacketFees: feesInEscrow.PacketFees,
+		}
+
+		identifiedFees = append(identifiedFees, identifiedFee)
 	}
 
 	return identifiedFees

diff --git a/modules/apps/29-fee/keeper/keeper_test.go b/modules/apps/29-fee/keeper/keeper_test.go
@@ -109,21 +109,25 @@ func (suite *KeeperTestSuite) TestGetAllIdentifiedPacketFees() {
 	}
 
 	// escrow the packet fee
-	identifiedPacketFee := types.NewIdentifiedPacketFee(packetID, fee, refundAcc.String(), []string{})
-	suite.chainA.GetSimApp().IBCFeeKeeper.SetFeeInEscrow(suite.chainA.GetContext(), identifiedPacketFee)
+	packetFee := types.NewPacketFee(fee, refundAcc.String(), []string{})
+	suite.chainA.GetSimApp().IBCFeeKeeper.SetFeesInEscrow(suite.chainA.GetContext(), packetID, types.NewPacketFees([]types.PacketFee{packetFee}))
 
-	expectedFees := []types.IdentifiedPacketFee{
+	expectedFees := []types.IdentifiedPacketFees{
 		{
-			PacketId:      packetID,
-			Fee:           fee,
-			RefundAddress: refundAcc.String(),
-			Relayers:      nil,
+			PacketId: packetID,
+			PacketFees: []types.PacketFee{
+				{
+					Fee:           fee,
+					RefundAddress: refundAcc.String(),
+					Relayers:      nil,
+				},
+			},
 		},
 	}
 
-	fees := suite.chainA.GetSimApp().IBCFeeKeeper.GetAllIdentifiedPacketFees(suite.chainA.GetContext())
-	suite.Require().Len(fees, len(expectedFees))
-	suite.Require().Equal(fees, expectedFees)
+	identifiedFees := suite.chainA.GetSimApp().IBCFeeKeeper.GetAllIdentifiedPacketFees(suite.chainA.GetContext())
+	suite.Require().Len(identifiedFees, len(expectedFees))
+	suite.Require().Equal(identifiedFees, expectedFees)
 }
 
 func (suite *KeeperTestSuite) TestGetAllFeeEnabledChannels() {

diff --git a/modules/apps/29-fee/types/fee.go b/modules/apps/29-fee/types/fee.go
@@ -16,6 +16,25 @@ func NewPacketFee(fee Fee, refundAddr string, relayers []string) PacketFee {
 	}
 }
 
+// Validate performs basic stateless validation of the associated PacketFee
+func (p PacketFee) Validate() error {
+	_, err := sdk.AccAddressFromBech32(p.RefundAddress)
+	if err != nil {
+		return sdkerrors.Wrap(err, "failed to convert RefundAddress into sdk.AccAddress")
+	}
+
+	// enforce relayer is nil
+	if p.Relayers != nil {
+		return ErrRelayersNotNil
+	}
+
+	if err := p.Fee.Validate(); err != nil {
+		return err
+	}
+
+	return nil
+}
+
 // NewPacketFees creates and returns a new PacketFees struct including a list of type PacketFee
 func NewPacketFees(packetFees []PacketFee) PacketFees {
 	return PacketFees{

diff --git a/modules/apps/29-fee/types/genesis.go b/modules/apps/29-fee/types/genesis.go
@@ -10,7 +10,7 @@ import (
 )
 
 // NewGenesisState creates a 29-fee GenesisState instance.
-func NewGenesisState(identifiedFees []IdentifiedPacketFee, feeEnabledChannels []FeeEnabledChannel, registeredRelayers []RegisteredRelayerAddress, forwardRelayers []ForwardRelayerAddress) *GenesisState {
+func NewGenesisState(identifiedFees []IdentifiedPacketFees, feeEnabledChannels []FeeEnabledChannel, registeredRelayers []RegisteredRelayerAddress, forwardRelayers []ForwardRelayerAddress) *GenesisState {
 	return &GenesisState{
 		IdentifiedFees:     identifiedFees,
 		FeeEnabledChannels: feeEnabledChannels,
@@ -22,7 +22,7 @@ func NewGenesisState(identifiedFees []IdentifiedPacketFee, feeEnabledChannels []
 // DefaultGenesisState returns a GenesisState with "transfer" as the default PortID.
 func DefaultGenesisState() *GenesisState {
 	return &GenesisState{
-		IdentifiedFees:     []IdentifiedPacketFee{},
+		IdentifiedFees:     []IdentifiedPacketFees{},
 		ForwardRelayers:    []ForwardRelayerAddress{},
 		FeeEnabledChannels: []FeeEnabledChannel{},
 		RegisteredRelayers: []RegisteredRelayerAddress{},
@@ -33,11 +33,16 @@ func DefaultGenesisState() *GenesisState {
 // failure.
 func (gs GenesisState) Validate() error {
 	// Validate IdentifiedPacketFees
-	for _, fee := range gs.IdentifiedFees {
-		err := fee.Validate()
-		if err != nil {
+	for _, identifiedFees := range gs.IdentifiedFees {
+		if err := identifiedFees.PacketId.Validate(); err != nil {
 			return err
 		}
+
+		for _, packetFee := range identifiedFees.PacketFees {
+			if err := packetFee.Validate(); err != nil {
+				return err
+			}
+		}
 	}
 
 	// Validate FeeEnabledChannels

diff --git a/modules/apps/29-fee/types/genesis.pb.go b/modules/apps/29-fee/types/genesis.pb.go
diff --git a/modules/apps/29-fee/types/genesis_test.go b/modules/apps/29-fee/types/genesis_test.go
@@ -168,12 +168,16 @@ func TestValidateGenesis(t *testing.T) {
 		tc.malleate()
 
 		genState := types.GenesisState{
-			IdentifiedFees: []types.IdentifiedPacketFee{
+			IdentifiedFees: []types.IdentifiedPacketFees{
 				{
-					PacketId:      packetId,
-					Fee:           fee,
-					RefundAddress: refundAcc,
-					Relayers:      nil,
+					PacketId: packetId,
+					PacketFees: []types.PacketFee{
+						{
+							Fee:           fee,
+							RefundAddress: refundAcc,
+							Relayers:      nil,
+						},
+					},
 				},
 			},
 			FeeEnabledChannels: []types.FeeEnabledChannel{

diff --git a/proto/ibc/applications/fee/v1/ack.proto b/proto/ibc/applications/fee/v1/ack.proto
@@ -11,5 +11,5 @@ import "gogoproto/gogo.proto";
 message IncentivizedAcknowledgement {
   bytes  result                  = 1;
   string forward_relayer_address = 2 [(gogoproto.moretags) = "yaml:\"forward_relayer_address\""];
-  bool underlying_app_success = 3 [(gogoproto.moretags) = "yaml:\"underlying_app_successl\""];
+  bool   underlying_app_success  = 3 [(gogoproto.moretags) = "yaml:\"underlying_app_successl\""];
 }