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.

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

feat: allow multiple addrs to incentivize packets #915

Merged
merged 12 commits into from
Feb 16, 2022
Merged
16 changes: 16 additions & 0 deletions docs/ibc/proto-docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
- [ibc/applications/fee/v1/fee.proto](#ibc/applications/fee/v1/fee.proto)
- [Fee](#ibc.applications.fee.v1.Fee)
- [IdentifiedPacketFee](#ibc.applications.fee.v1.IdentifiedPacketFee)
- [IdentifiedPacketFees](#ibc.applications.fee.v1.IdentifiedPacketFees)

- [ibc/applications/fee/v1/genesis.proto](#ibc/applications/fee/v1/genesis.proto)
- [FeeEnabledChannel](#ibc.applications.fee.v1.FeeEnabledChannel)
Expand Down Expand Up @@ -736,6 +737,21 @@ 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


| Field | Type | Label | Description |
| ----- | ---- | ----- | ----------- |
| `packet_fees` | [IdentifiedPacketFee](#ibc.applications.fee.v1.IdentifiedPacketFee) | repeated | |





<!-- end messages -->

<!-- end enums -->
Expand Down
20 changes: 12 additions & 8 deletions modules/apps/29-fee/ibc_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,15 +224,17 @@ func (im IBCModule) OnAcknowledgementPacket(
return sdkerrors.Wrapf(err, "cannot unmarshal ICS-29 incentivized packet acknowledgement: %v", ack)
}

packetId := channeltypes.NewPacketId(packet.SourceChannel, packet.SourcePort, packet.Sequence)

identifiedPacketFee, found := im.keeper.GetFeeInEscrow(ctx, packetId)
packetID := channeltypes.NewPacketId(packet.SourceChannel, packet.SourcePort, packet.Sequence)
identifiedPacketFees, 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, identifiedPacketFee.RefundAddress, ack.ForwardRelayerAddress, relayer, identifiedPacketFee)
im.keeper.DistributePacketFees(ctx, ack.ForwardRelayerAddress, relayer, identifiedPacketFees.PacketFees)

// removes the fees from the store as fees are now paid
im.keeper.DeleteFeesInEscrow(ctx, packetID)

// call underlying callback
return im.app.OnAcknowledgementPacket(ctx, packet, ack.Result, relayer)
Expand All @@ -249,15 +251,17 @@ func (im IBCModule) OnTimeoutPacket(
return im.app.OnTimeoutPacket(ctx, packet, relayer)
}

packetId := channeltypes.NewPacketId(packet.SourceChannel, packet.SourcePort, packet.Sequence)

identifiedPacketFee, found := im.keeper.GetFeeInEscrow(ctx, packetId)
packetID := channeltypes.NewPacketId(packet.SourceChannel, packet.SourcePort, packet.Sequence)
identifiedPacketFees, 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, identifiedPacketFee.RefundAddress, relayer, identifiedPacketFee)
im.keeper.DistributePacketFeesOnTimeout(ctx, relayer, identifiedPacketFees.PacketFees)

// removes the fee from the store as fee is now paid
im.keeper.DeleteFeesInEscrow(ctx, packetID)

// call underlying callback
return im.app.OnTimeoutPacket(ctx, packet, relayer)
Expand Down
16 changes: 8 additions & 8 deletions modules/apps/29-fee/ibc_module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,7 @@ func (suite *FeeTestSuite) TestOnAcknowledgementPacket() {
"no op success without a packet fee",
func() {
packetId := channeltypes.NewPacketId(suite.path.EndpointA.ChannelID, suite.path.EndpointA.ChannelConfig.PortID, suite.chainA.SenderAccount.GetSequence())
suite.chainA.GetSimApp().IBCFeeKeeper.DeleteFeeInEscrow(suite.chainA.GetContext(), packetId)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: The tests in these files could also use multiple fees

suite.chainA.GetSimApp().IBCFeeKeeper.DeleteFeesInEscrow(suite.chainA.GetContext(), packetId)

ack = types.IncentivizedAcknowledgement{
Result: channeltypes.NewResultAcknowledgement([]byte{1}).Acknowledgement(),
Expand Down Expand Up @@ -620,8 +620,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(identifiedFee.Fee.TimeoutFee[0])

// malleate test case
tc.malleate()
Expand Down Expand Up @@ -680,7 +679,7 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() {
func() {
// delete packet fee
packetId := channeltypes.NewPacketId(suite.path.EndpointA.ChannelID, suite.path.EndpointA.ChannelConfig.PortID, suite.chainA.SenderAccount.GetSequence())
suite.chainA.GetSimApp().IBCFeeKeeper.DeleteFeeInEscrow(suite.chainA.GetContext(), packetId)
suite.chainA.GetSimApp().IBCFeeKeeper.DeleteFeesInEscrow(suite.chainA.GetContext(), packetId)

expectedBalance = originalBalance.Add(ibctesting.TestCoin) // timeout refund for ics20 transfer
},
Expand Down Expand Up @@ -765,10 +764,11 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() {

relayerBalance := sdk.NewCoins(suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), relayerAddr, ibctesting.TestCoin.Denom))
if tc.expFeeDistributed {
suite.Require().Equal(
identifiedFee.Fee.TimeoutFee,
relayerBalance,
)
// there should no longer be a fee in escrow for this packet
found := suite.chainA.GetSimApp().IBCFeeKeeper.HasFeesInEscrow(suite.chainA.GetContext(), packetId)
suite.Require().False(found)

suite.Require().Equal(identifiedFee.Fee.TimeoutFee, relayerBalance)
} else {
suite.Require().Empty(relayerBalance)
}
Expand Down
127 changes: 68 additions & 59 deletions modules/apps/29-fee/keeper/escrow.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,63 +26,69 @@ func (k Keeper) EscrowPacketFee(ctx sdk.Context, identifiedFee types.IdentifiedP
return sdkerrors.Wrapf(types.ErrRefundAccNotFound, "account with address: %s not found", refundAcc)
}

coins := identifiedFee.Fee.RecvFee
coins = coins.Add(identifiedFee.Fee.AckFee...)
coins = coins.Add(identifiedFee.Fee.TimeoutFee...)

if err := k.bankKeeper.SendCoinsFromAccountToModule(
ctx, refundAcc, types.ModuleName, coins,
); err != nil {
coins := identifiedFee.Fee.Total()
if err := k.bankKeeper.SendCoinsFromAccountToModule(ctx, refundAcc, types.ModuleName, coins); err != nil {
return err
}

// Store fee in state for reference later
k.SetFeeInEscrow(ctx, identifiedFee)
packetFees := []types.IdentifiedPacketFee{identifiedFee}
if feesInEscrow, found := k.GetFeesInEscrow(ctx, identifiedFee.PacketId); found {
packetFees = append(packetFees, feesInEscrow.PacketFees...)
}

identifiedFees := types.NewIdentifiedPacketFees(packetFees)
k.SetFeesInEscrow(ctx, identifiedFee.PacketId, identifiedFees)
AdityaSripal marked this conversation as resolved.
Show resolved Hide resolved

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, refundAcc, forwardRelayer string, reverseRelayer sdk.AccAddress, feeInEscrow types.IdentifiedPacketFee) {
func (k Keeper) DistributePacketFees(ctx sdk.Context, forwardRelayer string, reverseRelayer sdk.AccAddress, feesInEscrow []types.IdentifiedPacketFee) {
// distribute fee for forward relaying
forward, err := sdk.AccAddressFromBech32(forwardRelayer)
if err == nil {
k.distributeFee(ctx, forward, feeInEscrow.Fee.RecvFee)
}
forwardAddr, fErr := sdk.AccAddressFromBech32(forwardRelayer)

// distribute fee for reverse relaying
k.distributeFee(ctx, reverseRelayer, feeInEscrow.Fee.AckFee)
for _, packetFee := range feesInEscrow {
if fErr == nil {
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
k.distributeFee(ctx, forwardAddr, packetFee.Fee.RecvFee)
}

// refund timeout fee refund,
refundAddr, err := sdk.AccAddressFromBech32(refundAcc)
if err != nil {
panic(fmt.Sprintf("could not parse refundAcc %s to sdk.AccAddress", refundAcc))
}
// distribute fee for reverse relaying
k.distributeFee(ctx, reverseRelayer, packetFee.Fee.AckFee)

// refund timeout fee for unused timeout
k.distributeFee(ctx, refundAddr, feeInEscrow.Fee.TimeoutFee)
// refund timeout fee refund,
refundAddr, err := sdk.AccAddressFromBech32(packetFee.RefundAddress)
AdityaSripal marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
panic(fmt.Sprintf("could not parse refundAcc %s to sdk.AccAddress", packetFee.RefundAddress))
}
colin-axner marked this conversation as resolved.
Show resolved Hide resolved

// removes the fee from the store as fee is now paid
k.DeleteFeeInEscrow(ctx, feeInEscrow.PacketId)
}
if fErr != nil {
// refund onRecv fee as forward relayer is not valid address
k.distributeFee(ctx, refundAddr, packetFee.Fee.RecvFee)
}

// 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, refundAcc string, timeoutRelayer sdk.AccAddress, feeInEscrow types.IdentifiedPacketFee) {
refundAddr, err := sdk.AccAddressFromBech32(refundAcc)
if err != nil {
panic(fmt.Sprintf("could not parse refundAcc %s to sdk.AccAddress", refundAcc))
// refund timeout fee for unused timeout
k.distributeFee(ctx, refundAddr, packetFee.Fee.TimeoutFee)
}
}

// refund receive fee for unused forward relaying
k.distributeFee(ctx, refundAddr, feeInEscrow.Fee.RecvFee)
// 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) {
for _, feeInEscrow := range feesInEscrow {
// check if refundAcc address works
refundAddr, err := sdk.AccAddressFromBech32(feeInEscrow.RefundAddress)
damiannolan marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
panic(fmt.Sprintf("could not parse refundAcc %s to sdk.AccAddress", feeInEscrow.RefundAddress))
}

// refund ack fee for unused reverse relaying
k.distributeFee(ctx, refundAddr, feeInEscrow.Fee.AckFee)
// refund receive fee for unused forward relaying
k.distributeFee(ctx, refundAddr, feeInEscrow.Fee.RecvFee)

// distribute fee for timeout relaying
k.distributeFee(ctx, timeoutRelayer, feeInEscrow.Fee.TimeoutFee)
// refund ack fee for unused reverse relaying
k.distributeFee(ctx, refundAddr, feeInEscrow.Fee.AckFee)

// removes the fee from the store as fee is now paid
k.DeleteFeeInEscrow(ctx, feeInEscrow.PacketId)
// distribute fee for timeout relaying
k.distributeFee(ctx, timeoutRelayer, feeInEscrow.Fee.TimeoutFee)
}
}

// distributeFee will attempt to distribute the escrowed fee to the receiver address.
Expand All @@ -106,28 +112,31 @@ func (k Keeper) RefundFeesOnChannel(ctx sdk.Context, portID, channelID string) e

var refundErr error

k.IterateChannelFeesInEscrow(ctx, portID, channelID, func(identifiedFee types.IdentifiedPacketFee) (stop bool) {
refundAccAddr, err := sdk.AccAddressFromBech32(identifiedFee.RefundAddress)
if err != nil {
refundErr = err
return true
k.IterateIdentifiedChannelFeesInEscrow(ctx, portID, channelID, func(identifiedFees types.IdentifiedPacketFees) (stop bool) {
for _, identifiedFee := range identifiedFees.PacketFees {
refundAccAddr, err := sdk.AccAddressFromBech32(identifiedFee.RefundAddress)
if err != nil {
refundErr = err
return true
}

// refund all fees to refund address
// Use SendCoins rather than the module account send functions since refund address may be a user account or module address.
// if any `SendCoins` call returns an error, we return error and stop iteration
if err = k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, refundAccAddr, identifiedFee.Fee.RecvFee); err != nil {
refundErr = err
return true
}
if err = k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, refundAccAddr, identifiedFee.Fee.AckFee); err != nil {
refundErr = err
return true
}
if err = k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, refundAccAddr, identifiedFee.Fee.TimeoutFee); err != nil {
refundErr = err
return true
}
}

// refund all fees to refund address
// Use SendCoins rather than the module account send functions since refund address may be a user account or module address.
// if any `SendCoins` call returns an error, we return error and stop iteration
if err = k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, refundAccAddr, identifiedFee.Fee.RecvFee); err != nil {
refundErr = err
return true
}
if err = k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, refundAccAddr, identifiedFee.Fee.AckFee); err != nil {
refundErr = err
return true
}
if err = k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, refundAccAddr, identifiedFee.Fee.TimeoutFee); err != nil {
refundErr = err
return true
}
return false
})

Expand Down
Loading