Fee Middleware: Escrow logic #465
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,128 @@ | ||||||
| package keeper | ||||||
|
|
||||||
| import ( | ||||||
| "fmt" | ||||||
|
|
||||||
| sdk "github.com/cosmos/cosmos-sdk/types" | ||||||
| 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" | ||||||
| ) | ||||||
|
|
||||||
| // TODO: add optional relayers arr | ||||||
| func (k Keeper) EscrowPacketFee(ctx sdk.Context, refundAcc sdk.AccAddress, fee types.Fee, packetID channeltypes.PacketId) error { | ||||||
| // 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.String())) | ||||||
| } | ||||||
|
|
||||||
| fees := sdk.Coins{ | ||||||
| *fee.AckFee, *fee.ReceiveFee, *fee.TimeoutFee, | ||||||
| } | ||||||
|
|
||||||
| // check if refundAcc has balance for each fee | ||||||
colin-axner marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| for _, f := range fees { | ||||||
| hasBalance := k.bankKeeper.HasBalance(ctx, refundAcc, f) | ||||||
| if !hasBalance { | ||||||
| return sdkerrors.Wrap(types.ErrBalanceNotFound, fmt.Sprintf("%s", refundAcc.String())) | ||||||
|
There was a problem hiding this comment.
Suggested change
There was a problem hiding this comment. Should this error be insufficient balance? |
||||||
| } | ||||||
| } | ||||||
|
|
||||||
| for _, coin := range fees { | ||||||
| // escrow each fee with account module | ||||||
| if err := k.bankKeeper.SendCoinsFromAccountToModule( | ||||||
AdityaSripal marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| ctx, refundAcc, types.ModuleName, sdk.Coins{coin}, | ||||||
| ); err != nil { | ||||||
| return err | ||||||
colin-axner marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| } | ||||||
| } | ||||||
|
There was a problem hiding this comment. We can be more capital efficient here. Really we just need to escrow the Since the packet will only go down one of those 2 paths. There's never a point where all 3 fees are paid out There was a problem hiding this comment. I was under the impression that each coin could be of a separate denom? There was a problem hiding this comment. That is correct. You can still calculate max. Suppose: Then you need to escrow: If packet gets acked, then you refund: If packet times out, you refund Same logic would hold for sdk.Coins fee There was a problem hiding this comment. You can check if there's a utility function in There was a problem hiding this comment. Okay, thanks, makes sense. There was a problem hiding this comment. I'd prefer to go with the simple approach and do this as a future optimization (like how we pushed consensus state pruning until after launch). It is additional complexity There was a problem hiding this comment. Agree with @colin-axner 👍 |
||||||
|
|
||||||
| // Store fee in state for reference later | ||||||
| // feeInEscrow/<refund-account>/<channel-id>/packet/<sequence-id>/ -> Fee (timeout, ack, onrecv) | ||||||
| k.SetFeeInEscrow(ctx, fee, refundAcc.String(), packetID.ChannelId, packetID.Sequence) | ||||||
| return nil | ||||||
| } | ||||||
|
|
||||||
| // PayFee 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) payFee(ctx sdk.Context, refundAcc, forwardRelayer, reverseRelayer sdk.AccAddress, packetID channeltypes.PacketId) error { | ||||||
| // check if there is a Fee in escrow for the given packetId | ||||||
| feeInEscrow, hasFee := k.GetFeeInEscrow(ctx, refundAcc.String(), packetID.ChannelId, packetID.Sequence) | ||||||
| if !hasFee { | ||||||
| return sdkerrors.Wrap(types.ErrFeeNotFound, fmt.Sprintf("%s", refundAcc.String())) | ||||||
|
There was a problem hiding this comment. Shouldn't the error indicate for the channel/sequence? |
||||||
| } | ||||||
|
|
||||||
| // get module accAddr | ||||||
| feeModuleAccAddr := k.authKeeper.GetModuleAddress(types.ModuleName) | ||||||
|
|
||||||
| // send ack fee to reverse relayer | ||||||
| if feeInEscrow.AckFee != nil { | ||||||
| err := k.bankKeeper.SendCoins(ctx, feeModuleAccAddr, reverseRelayer, sdk.Coins{*feeInEscrow.AckFee}) | ||||||
| if err != nil { | ||||||
| return sdkerrors.Wrap(types.ErrPayingFee, fmt.Sprintf("Error sending coin with Denom: %s for Amount: %d", feeInEscrow.AckFee.Denom, feeInEscrow.AckFee.Amount)) | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| // send receive fee to forward relayer | ||||||
| if feeInEscrow.ReceiveFee != nil { | ||||||
| err := k.bankKeeper.SendCoins(ctx, feeModuleAccAddr, forwardRelayer, sdk.Coins{*feeInEscrow.ReceiveFee}) | ||||||
| if err != nil { | ||||||
| return sdkerrors.Wrap(types.ErrPayingFee, fmt.Sprintf("Error sending coin with Denom: %s for Amount: %d", feeInEscrow.ReceiveFee.Denom, feeInEscrow.ReceiveFee.Amount)) | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| // refund timeout fee to refundAddr | ||||||
| if feeInEscrow.TimeoutFee != nil { | ||||||
| err := k.bankKeeper.SendCoins(ctx, feeModuleAccAddr, refundAcc, sdk.Coins{*feeInEscrow.TimeoutFee}) | ||||||
| if err != nil { | ||||||
| return sdkerrors.Wrap(types.ErrRefundingFee, fmt.Sprintf("Error sending coin with Denom: %s for Amount: %d", feeInEscrow.TimeoutFee.Denom, feeInEscrow.TimeoutFee.Amount)) | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| // set fee as an empty struct (if we reach this point Fee is paid in full) | ||||||
| k.SetFeeInEscrow(ctx, types.Fee{}, refundAcc.String(), packetID.ChannelId, packetID.Sequence) | ||||||
|
|
||||||
| return nil | ||||||
| } | ||||||
|
|
||||||
| // PayFeeTimeout 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) payFeeTimeout(ctx sdk.Context, refundAcc, reverseRelayer sdk.AccAddress, packetID channeltypes.PacketId) error { | ||||||
| // check if there is a Fee in escrow for the given packetId | ||||||
| feeInEscrow, hasFee := k.GetFeeInEscrow(ctx, refundAcc.String(), packetID.ChannelId, packetID.Sequence) | ||||||
| if !hasFee { | ||||||
| return sdkerrors.Wrap(types.ErrFeeNotFound, fmt.Sprintf("%s", refundAcc.String())) | ||||||
| } | ||||||
|
|
||||||
| // get module accAddr | ||||||
| feeModuleAccAddr := k.authKeeper.GetModuleAddress(types.ModuleName) | ||||||
|
|
||||||
| // refund the ack fee | ||||||
| if feeInEscrow.AckFee != nil { | ||||||
| err := k.bankKeeper.SendCoins(ctx, feeModuleAccAddr, refundAcc, sdk.Coins{*feeInEscrow.AckFee}) | ||||||
| if err != nil { | ||||||
| return sdkerrors.Wrap(types.ErrRefundingFee, fmt.Sprintf("Error sending coin with Denom: %s for Amount: %d", feeInEscrow.AckFee.Denom, feeInEscrow.AckFee.Amount)) | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| // refund the receive fee | ||||||
| if feeInEscrow.ReceiveFee != nil { | ||||||
| err := k.bankKeeper.SendCoins(ctx, feeModuleAccAddr, refundAcc, sdk.Coins{*feeInEscrow.ReceiveFee}) | ||||||
| if err != nil { | ||||||
| return sdkerrors.Wrap(types.ErrRefundingFee, fmt.Sprintf("Error sending coin with Denom: %s for Amount: %d", feeInEscrow.ReceiveFee.Denom, feeInEscrow.ReceiveFee.Amount)) | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| // pay the timeout fee to the reverse relayer | ||||||
| if feeInEscrow.TimeoutFee != nil { | ||||||
| err := k.bankKeeper.SendCoins(ctx, feeModuleAccAddr, reverseRelayer, sdk.Coins{*feeInEscrow.TimeoutFee}) | ||||||
| if err != nil { | ||||||
| return sdkerrors.Wrap(types.ErrPayingFee, fmt.Sprintf("Error sending coin with Denom: %s for Amount: %d", feeInEscrow.TimeoutFee.Denom, feeInEscrow.TimeoutFee.Amount)) | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| // set fee as an empty struct (if we reach this point Fee is paid in full) | ||||||
| k.SetFeeInEscrow(ctx, types.Fee{}, refundAcc.String(), packetID.ChannelId, packetID.Sequence) | ||||||
|
|
||||||
| return nil | ||||||
| } | ||||||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| package keeper_test | ||
|
|
||
| import ( | ||
| sdk "github.com/cosmos/cosmos-sdk/types" | ||
| "github.com/cosmos/ibc-go/modules/apps/29-fee/types" | ||
| channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types" | ||
seantking marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ) | ||
|
|
||
| func (suite *KeeperTestSuite) TestEscrowPacketFee() { | ||
| var ( | ||
| err error | ||
| ) | ||
|
|
||
| testCases := []struct { | ||
AdityaSripal marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| name string | ||
| malleate func() | ||
| expPass bool | ||
| }{ | ||
| { | ||
| "success", func() {}, true, | ||
| }, | ||
| } | ||
|
|
||
| for _, tc := range testCases { | ||
| tc := tc | ||
|
|
||
| suite.Run(tc.name, func() { | ||
| suite.SetupTest() // reset | ||
|
|
||
| // setup | ||
| refundAcc := suite.chainA.SenderAccount.GetAddress() | ||
| ackFee := &sdk.Coin{Denom: "stake", Amount: sdk.NewInt(100)} | ||
| receiveFee := &sdk.Coin{Denom: "stake", Amount: sdk.NewInt(100)} | ||
| timeoutFee := &sdk.Coin{Denom: "stake", Amount: sdk.NewInt(100)} | ||
| fee := types.Fee{ackFee, receiveFee, timeoutFee} | ||
| packetId := channeltypes.PacketId{ChannelId: "channel-0", PortId: "fee", Sequence: uint64(1)} | ||
|
|
||
| tc.malleate() | ||
|
|
||
| // escrow the packet fee | ||
| err = suite.chainA.GetSimApp().IBCFeeKeeper.EscrowPacketFee(suite.chainA.GetContext(), refundAcc, fee, packetId) | ||
|
|
||
| if tc.expPass { | ||
| suite.Require().NoError(err) | ||
| } else { | ||
| suite.Require().Error(err) | ||
| } | ||
|
|
||
| }) | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if %s is used, .String() should automatically be called (unless I am mistaken?)