Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: make fee storage more efficient #956

Merged
merged 7 commits into from
Feb 23, 2022
Merged

refactor: make fee storage more efficient #956

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
23940d4
adding new proto types and codegen
damiannolan Feb 21, 2022
615200b
refactoring ics29 fees for more efficient storage
damiannolan Feb 21, 2022
c5178be
updating tests
damiannolan Feb 21, 2022
e3bd5c5
fixing typo in protodoc comments
damiannolan Feb 21, 2022
83719ee
Merge branch 'ics29-fee-middleware' into damian/760-effecient-storage
damiannolan Feb 21, 2022
0555a08
Merge branch 'ics29-fee-middleware' into damian/760-effecient-storage
damiannolan Feb 23, 2022
c7455ef
Merge branch 'ics29-fee-middleware' into damian/760-effecient-storage
damiannolan Feb 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 38 additions & 2 deletions docs/ibc/proto-docs.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 | |



Expand Down
8 changes: 4 additions & 4 deletions modules/apps/29-fee/ibc_module.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
46 changes: 22 additions & 24 deletions modules/apps/29-fee/ibc_module_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)
Expand Down Expand Up @@ -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,
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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,
},
Expand Down Expand Up @@ -606,7 +606,7 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() {
ForwardRelayerAddress: blockedAddr.String(),
}.Acknowledgement()

expectedRelayerBalance = identifiedFee.Fee.AckFee
expectedRelayerBalance = packetFee.Fee.AckFee
},
true,
},
Expand All @@ -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,
Expand All @@ -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()
Expand All @@ -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()
Expand Down Expand Up @@ -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
)
Expand Down Expand Up @@ -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,
},
Expand All @@ -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,
Expand All @@ -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
Expand All @@ -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()
Expand All @@ -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)
}
Expand Down
24 changes: 12 additions & 12 deletions modules/apps/29-fee/keeper/escrow.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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 {
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down
Loading