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

Fee Middleware: Escrow logic #465

Merged
merged 50 commits into from
Nov 5, 2021
Merged
Show file tree
Hide file tree
Changes from 44 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
435efc7
fix: adding second endpoint for async pay fee + renaming types
seantking Oct 1, 2021
8f2c51a
feat: adding escrow logic
seantking Oct 1, 2021
a410af6
feat: updating proto types & escrow logic
seantking Oct 4, 2021
0e6823f
fix: stub fn & proto comment
seantking Oct 5, 2021
f41e520
feat: adding PayFee & PayFeeTimeout & escrow_test
seantking Oct 6, 2021
e4bd49a
test: adding happy path for EscrowPacketFee
seantking Oct 6, 2021
ca7f4e2
fix: comments, error handling
seantking Oct 7, 2021
6a5add7
fix: comments & grammar
seantking Oct 7, 2021
535e2c6
test: adding unhappy path for escrow
seantking Oct 8, 2021
5bd0e32
tests(escrow): adding hasBalance check for module acc
seantking Oct 8, 2021
31fcee3
test(PayFee): adding happy path for PayFee tests
seantking Oct 8, 2021
487f78d
tests(PayFee, PayFeeTimeout): adding tests
seantking Oct 8, 2021
f25932f
fix: adding relayers back to IdentifiedPacket
seantking Oct 11, 2021
38566bc
fix: removing refund acc from key
seantking Oct 11, 2021
dd0044b
fix: storing IdentifiedPacketFee in state instead of Fee
seantking Oct 11, 2021
5b4a395
feat: adding msg_server test for registerCPAddr, wiring for codec + s…
seantking Oct 11, 2021
c9af582
test: adding msg_server test for PayPacketFee
seantking Oct 12, 2021
d325146
test: adding PayPacketFeeAsync msg_server test
seantking Oct 12, 2021
ea9b30b
chore: updating PayFee -> DistributeFee & minor nits
seantking Oct 13, 2021
ac7584f
nit: removing unnecessary nil check
seantking Oct 13, 2021
c44a81c
refactor: add portId to store key & use packetId as param
seantking Oct 13, 2021
a8c3cff
fix: add DeleteFeeInEscrow & remove fee on successful distribution
seantking Oct 13, 2021
4f490ea
tests: adding validation & signer tests for PayFee/Async & updating p…
seantking Oct 13, 2021
f1fde1d
chore: adding NewIdentifiedPacketFee fn
seantking Oct 13, 2021
eeb4376
fix: getter/setter for counterparty address + fix NewIdentifiedPacketFee
seantking Oct 14, 2021
4612bf2
fix: updating EscrowPacketFee with correct usage of coins api
seantking Oct 14, 2021
3494a0f
test: adding balance check for refund acc after escrow
seantking Oct 14, 2021
f4c37e6
fix: remove unncessary errors
seantking Oct 14, 2021
0b733ab
test: updating escrow tests + miscellaneous fixes
seantking Nov 3, 2021
2be4ab5
nit: updating var names
seantking Nov 3, 2021
cb2cbc3
docs: godoc
seantking Nov 3, 2021
66454c6
refactor: IdentifiedPacketFee & Fee no longer pointers
seantking Nov 3, 2021
06d4c25
fixes: small fixes
seantking Nov 3, 2021
f336ca7
Update modules/apps/29-fee/keeper/escrow.go
seantking Nov 4, 2021
8e2eb8f
Update modules/apps/29-fee/keeper/escrow.go
seantking Nov 4, 2021
52fa63e
Update modules/apps/29-fee/keeper/keeper.go
seantking Nov 4, 2021
0587a3c
Update modules/apps/29-fee/keeper/msg_server.go
seantking Nov 4, 2021
63e7a1a
Update modules/apps/29-fee/keeper/msg_server.go
seantking Nov 4, 2021
a40159f
Update modules/apps/29-fee/types/msgs.go
seantking Nov 4, 2021
f379aaf
nit: proto doc & error fix
seantking Nov 4, 2021
2f649a8
fix: escrow test
seantking Nov 5, 2021
b100ee0
test: updating distribute fee tests
seantking Nov 5, 2021
b19ebd2
test: adding validation check for fee and updating tests
seantking Nov 5, 2021
a51e2ff
test: allow counterparty address to be arbitrary string
seantking Nov 5, 2021
bbfd8d7
fix: message validation should pass if one fee is valid
seantking Nov 5, 2021
6640660
Update modules/apps/29-fee/keeper/escrow.go
seantking Nov 5, 2021
4610c99
Update modules/apps/29-fee/keeper/escrow.go
seantking Nov 5, 2021
bec52c0
fix: nits
seantking Nov 5, 2021
2c9185c
Update modules/apps/29-fee/keeper/escrow.go
seantking Nov 5, 2021
f4de334
test: adding isZero check for msgs
seantking Nov 5, 2021
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
68 changes: 49 additions & 19 deletions docs/ibc/proto-docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,10 @@
- [Query](#ibc.applications.fee.v1.Query)

- [ibc/applications/fee/v1/tx.proto](#ibc/applications/fee/v1/tx.proto)
- [MsgEscrowPacketFee](#ibc.applications.fee.v1.MsgEscrowPacketFee)
- [MsgEscrowPacketFeeResponse](#ibc.applications.fee.v1.MsgEscrowPacketFeeResponse)
- [MsgPayPacketFee](#ibc.applications.fee.v1.MsgPayPacketFee)
- [MsgPayPacketFeeAsync](#ibc.applications.fee.v1.MsgPayPacketFeeAsync)
- [MsgPayPacketFeeAsyncResponse](#ibc.applications.fee.v1.MsgPayPacketFeeAsyncResponse)
- [MsgPayPacketFeeResponse](#ibc.applications.fee.v1.MsgPayPacketFeeResponse)
- [MsgRegisterCounterpartyAddress](#ibc.applications.fee.v1.MsgRegisterCounterpartyAddress)
- [MsgRegisterCounterpartyAddressResponse](#ibc.applications.fee.v1.MsgRegisterCounterpartyAddressResponse)

Expand Down Expand Up @@ -628,7 +630,9 @@ https://github.com/cosmos/ibc/tree/master/spec/app/ics-029-fee-payment#fee-middl

| Field | Type | Label | Description |
| ----- | ---- | ----- | ----------- |
| `amount` | [cosmos.base.v1beta1.Coin](#cosmos.base.v1beta1.Coin) | repeated | |
| `receive_fee` | [cosmos.base.v1beta1.Coin](#cosmos.base.v1beta1.Coin) | repeated | |
| `ack_fee` | [cosmos.base.v1beta1.Coin](#cosmos.base.v1beta1.Coin) | repeated | |
| `timeout_fee` | [cosmos.base.v1beta1.Coin](#cosmos.base.v1beta1.Coin) | repeated | |



Expand All @@ -644,9 +648,7 @@ Fee associated with a packet_id
| Field | Type | Label | Description |
| ----- | ---- | ----- | ----------- |
| `packet_id` | [ibc.core.channel.v1.PacketId](#ibc.core.channel.v1.PacketId) | | |
| `receive_fee` | [Fee](#ibc.applications.fee.v1.Fee) | | |
| `ack_fee` | [Fee](#ibc.applications.fee.v1.Fee) | | |
| `timeout_fee` | [Fee](#ibc.applications.fee.v1.Fee) | | |
| `fee` | [Fee](#ibc.applications.fee.v1.Fee) | | |
| `relayers` | [string](#string) | repeated | |


Expand Down Expand Up @@ -676,11 +678,6 @@ Fee associated with a packet_id
GenesisState defines the fee middleware genesis state


| Field | Type | Label | Description |
| ----- | ---- | ----- | ----------- |
| `packets_fees` | [IdentifiedPacketFee](#ibc.applications.fee.v1.IdentifiedPacketFee) | repeated | A mapping of packets -> escrowed fees |





Expand Down Expand Up @@ -889,26 +886,58 @@ Query provides defines the gRPC querier service.



<a name="ibc.applications.fee.v1.MsgEscrowPacketFee"></a>
<a name="ibc.applications.fee.v1.MsgPayPacketFee"></a>

### MsgEscrowPacketFee
MsgEscrowPacketFee defines the request type EscrowPacketFee RPC
### MsgPayPacketFee
MsgPayPacketFee defines the request type PayPacketFee RPC
This Msg can be used to pay for a packet at the next sequence send & should be combined with the Msg that will be
paid for


| Field | Type | Label | Description |
| ----- | ---- | ----- | ----------- |
| `incentivized_packet` | [IdentifiedPacketFee](#ibc.applications.fee.v1.IdentifiedPacketFee) | | |
| `fee` | [Fee](#ibc.applications.fee.v1.Fee) | | |
| `source_port_id` | [string](#string) | | source channel port identifier |
| `source_channel_id` | [string](#string) | | source channel unique identifier |
| `signer` | [string](#string) | | account address to refund fee if necessary |
| `relayers` | [string](#string) | repeated | |






<a name="ibc.applications.fee.v1.MsgEscrowPacketFeeResponse"></a>
<a name="ibc.applications.fee.v1.MsgPayPacketFeeAsync"></a>

### MsgPayPacketFeeAsync
MsgPayPacketFeeAsync defines the request type PayPacketFeeAsync RPC
This Msg can be used to pay for a packet at a specified sequence (instead of the next sequence send)


| Field | Type | Label | Description |
| ----- | ---- | ----- | ----------- |
| `identified_packet_fee` | [IdentifiedPacketFee](#ibc.applications.fee.v1.IdentifiedPacketFee) | | packet to pay fee for |
| `signer` | [string](#string) | | account address to refund fee if necessary |






<a name="ibc.applications.fee.v1.MsgPayPacketFeeAsyncResponse"></a>

### MsgPayPacketFeeAsyncResponse
MsgPayPacketFeeAsyncResponse defines the response type for Msg/PayPacketFeeAsync






<a name="ibc.applications.fee.v1.MsgPayPacketFeeResponse"></a>

### MsgEscrowPacketFeeResponse
MsgEscrowPacketFeeResponse defines the response type for Msg/EscrowPacketFee
### MsgPayPacketFeeResponse
MsgPayPacketFeeResponse defines the response type for Msg/PayPacketFee



Expand Down Expand Up @@ -955,7 +984,8 @@ Msg defines the ibc/fee Msg service.
| Method Name | Request Type | Response Type | Description | HTTP Verb | Endpoint |
| ----------- | ------------ | ------------- | ------------| ------- | -------- |
| `RegisterCounterpartyAddress` | [MsgRegisterCounterpartyAddress](#ibc.applications.fee.v1.MsgRegisterCounterpartyAddress) | [MsgRegisterCounterpartyAddressResponse](#ibc.applications.fee.v1.MsgRegisterCounterpartyAddressResponse) | RegisterCounterpartyAddress defines a rpc handler method for MsgRegisterCounterpartyAddress RegisterCounterpartyAddress is called by the relayer on each channelEnd and allows them to specify their counterparty address before relaying. This ensures they will be properly compensated for forward relaying since destination chain must send back relayer's source address (counterparty address) in acknowledgement. This function may be called more than once by a relayer, in which case, latest counterparty address is always used. | |
| `EscrowPacketFee` | [MsgEscrowPacketFee](#ibc.applications.fee.v1.MsgEscrowPacketFee) | [MsgEscrowPacketFeeResponse](#ibc.applications.fee.v1.MsgEscrowPacketFeeResponse) | EscrowPacketFee defines a rpc handler method for MsgEscrowPacketFee EscrowPacketFee is an open callback that may be called by any module/user that wishes to escrow funds in order to incentivize the relaying of the given packet. | |
| `PayPacketFee` | [MsgPayPacketFee](#ibc.applications.fee.v1.MsgPayPacketFee) | [MsgPayPacketFeeResponse](#ibc.applications.fee.v1.MsgPayPacketFeeResponse) | PayPacketFee defines a rpc handler method for MsgPayPacketFee PayPacketFee is an open callback that may be called by any module/user that wishes to escrow funds in order to incentivize the relaying of the packet at the next sequence | |
| `PayPacketFeeAsync` | [MsgPayPacketFeeAsync](#ibc.applications.fee.v1.MsgPayPacketFeeAsync) | [MsgPayPacketFeeAsyncResponse](#ibc.applications.fee.v1.MsgPayPacketFeeAsyncResponse) | PayPacketFeeAsync defines a rpc handler method for MsgPayPacketFeeAsync PayPacketFeeAsync is an open callback that may be called by any module/user that wishes to escrow funds in order to incentivize the relaying of a known packet | |

<!-- end services -->

Expand Down
106 changes: 106 additions & 0 deletions modules/apps/29-fee/keeper/escrow.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
package keeper

import (
"fmt"

sdk "github.com/cosmos/cosmos-sdk/types"

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"

"github.com/cosmos/ibc-go/modules/apps/29-fee/types"
channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types"
)

// EscrowPacketFee sends the packet fee to the 29-fee module account to hold in escrow
func (k Keeper) EscrowPacketFee(ctx sdk.Context, refundAcc sdk.AccAddress, identifiedFee *types.IdentifiedPacketFee) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does identifiedFee need to be a pointer?

// check if the refund account exists
hasRefundAcc := k.authKeeper.GetAccount(ctx, refundAcc)
if hasRefundAcc == nil {
return sdkerrors.Wrap(types.ErrRefundAccNotFound, fmt.Sprintf("Account with address: %s not found", refundAcc))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return sdkerrors.Wrap(types.ErrRefundAccNotFound, fmt.Sprintf("Account with address: %s not found", refundAcc))
return sdkerrors.Wrapf(types.ErrRefundAccNotFound, "account with address: %s not found", refundAcc)

}

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

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

// Store fee in state for reference later
// feeInEscrow/<port-id>/<channel-id>/packet/<sequence-id>/ -> Fee (timeout, ack, onrecv)
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be removed imo. If we change the storage path, we will probably forget to remove this. We can indicate the storage path in the SetFeeInEscrow godoc if necessary

k.SetFeeInEscrow(ctx, identifiedFee)
return nil
}

// DistributeFee 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) DistributeFee(ctx sdk.Context, refundAcc, forwardRelayer, reverseRelayer sdk.AccAddress, packetID *channeltypes.PacketId) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto on PacketId being a pointer?

// check if there is a Fee in escrow for the given packetId
feeInEscrow, found := k.GetFeeInEscrow(ctx, packetID)
if !found {
return sdkerrors.Wrap(types.ErrFeeNotFound, fmt.Sprintf("with channelID: %s, sequenceId %d", packetID.ChannelId, packetID.Sequence))
seantking marked this conversation as resolved.
Show resolved Hide resolved
}

// get module accAddr
feeModuleAccAddr := k.authKeeper.GetModuleAddress(types.ModuleName)

// send receive fee to forward relayer
err := k.bankKeeper.SendCoins(ctx, feeModuleAccAddr, forwardRelayer, feeInEscrow.Fee.ReceiveFee)
if err != nil {
return sdkerrors.Wrap(err, "error sending fee to forward relayer")
seantking marked this conversation as resolved.
Show resolved Hide resolved
}

// send ack fee to reverse relayer
err = k.bankKeeper.SendCoins(ctx, feeModuleAccAddr, reverseRelayer, feeInEscrow.Fee.AckFee)
if err != nil {
return sdkerrors.Wrap(err, "error sending fee to reverse relayer")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return sdkerrors.Wrap(err, "error sending fee to reverse relayer")
return sdkerrors.Wrap(err, "failed to send fee to reverse relayer")

}

// refund timeout fee to refundAddr
err = k.bankKeeper.SendCoins(ctx, feeModuleAccAddr, refundAcc, feeInEscrow.Fee.TimeoutFee)
if err != nil {
return sdkerrors.Wrap(err, "error refunding timeout fee")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return sdkerrors.Wrap(err, "error refunding timeout fee")
return sdkerrors.Wrap(err, "failed to refund timeout fee")

}

// removes the fee from the store as fee is now paid
k.DeleteFeeInEscrow(ctx, packetID)

return nil
}

// DistributeFeeTimeout 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) DistributeFeeTimeout(ctx sdk.Context, refundAcc, timeoutRelayer sdk.AccAddress, packetID *channeltypes.PacketId) error {
// check if there is a Fee in escrow for the given packetId
feeInEscrow, hasFee := k.GetFeeInEscrow(ctx, packetID)
if !hasFee {
return sdkerrors.Wrap(types.ErrFeeNotFound, fmt.Sprintf("%s", refundAcc.String()))
}
seantking marked this conversation as resolved.
Show resolved Hide resolved

// get module accAddr
feeModuleAccAddr := k.authKeeper.GetModuleAddress(types.ModuleName)

// refund the receive fee
err := k.bankKeeper.SendCoins(ctx, feeModuleAccAddr, refundAcc, feeInEscrow.Fee.ReceiveFee)
if err != nil {
return sdkerrors.Wrap(err, "error refunding receive fee")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return sdkerrors.Wrap(err, "error refunding receive fee")
return sdkerrors.Wrap(err, "failed to refund receive fee")

}

// refund the ack fee
err = k.bankKeeper.SendCoins(ctx, feeModuleAccAddr, refundAcc, feeInEscrow.Fee.AckFee)
if err != nil {
return sdkerrors.Wrap(err, "error refunding ack fee")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return sdkerrors.Wrap(err, "error refunding ack fee")
return sdkerrors.Wrap(err, "failed to refund ack fee")

}

// pay the timeout fee to the reverse relayer
err = k.bankKeeper.SendCoins(ctx, feeModuleAccAddr, timeoutRelayer, feeInEscrow.Fee.TimeoutFee)
if err != nil {
return sdkerrors.Wrap(err, "error sending fee to timeout relayer")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return sdkerrors.Wrap(err, "error sending fee to timeout relayer")
return sdkerrors.Wrap(err, "failed to send fee to timeout relayer")

}

// removes the fee from the store as fee is now paid
k.DeleteFeeInEscrow(ctx, packetID)

return nil
}
Loading