Skip to content

Commit

Permalink
Merge branch 'ics29-fee-middleware' into damian/955-genesis-state-update
Browse files Browse the repository at this point in the history
  • Loading branch information
damiannolan committed Feb 23, 2022
2 parents 200ca36 + b02d193 commit c4e6f41
Show file tree
Hide file tree
Showing 31 changed files with 547 additions and 257 deletions.
5 changes: 4 additions & 1 deletion docs/ibc/proto-docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,7 @@ It contains the raw acknowledgement bytes, as well as the forward relayer addres
| ----- | ---- | ----- | ----------- |
| `result` | [bytes](#bytes) | | |
| `forward_relayer_address` | [string](#string) | | |
| `underlying_app_success` | [bool](#bool) | | |



Expand Down Expand Up @@ -743,7 +744,7 @@ 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 PacketFree and associated PacketId
IdentifiedPacketFees contains a list of type PacketFee and associated PacketId


| Field | Type | Label | Description |
Expand Down Expand Up @@ -865,6 +866,7 @@ RegisteredRelayerAddress contains the address and counterparty address for a spe
| ----- | ---- | ----- | ----------- |
| `address` | [string](#string) | | |
| `counterparty_address` | [string](#string) | | |
| `channel_id` | [string](#string) | | |



Expand Down Expand Up @@ -1076,6 +1078,7 @@ MsgRegisterCounterpartyAddress is the request type for registering the counterpa
| ----- | ---- | ----- | ----------- |
| `address` | [string](#string) | | |
| `counterparty_address` | [string](#string) | | |
| `channel_id` | [string](#string) | | |



Expand Down
4 changes: 2 additions & 2 deletions modules/apps/29-fee/client/cli/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,14 @@ func GetQueryCmd() *cobra.Command {
func NewTxCmd() *cobra.Command {
txCmd := &cobra.Command{
Use: "ibc-fee",
Short: "", // TODO
Short: "Transaction subcommand for IBC relayer incentivization",
DisableFlagParsing: true,
SuggestionsMinimumDistance: 2,
RunE: client.ValidateCmd,
}

txCmd.AddCommand(
// TODO
NewPayPacketFeeAsyncTxCmd(),
)

return txCmd
Expand Down
98 changes: 97 additions & 1 deletion modules/apps/29-fee/client/cli/tx.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,99 @@
package cli

// TODO
import (
"fmt"
"strconv"
"strings"

"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/flags"
"github.com/cosmos/cosmos-sdk/client/tx"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/version"
"github.com/spf13/cobra"

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

const (
flagRecvFee = "recv-fee"
flagAckFee = "ack-fee"
flagTimeoutFee = "timeout-fee"
)

// NewPayPacketFeeAsyncTxCmd returns the command to create a MsgPayPacketFeeAsync
func NewPayPacketFeeAsyncTxCmd() *cobra.Command {
cmd := &cobra.Command{
Use: "pay-packet-fee [src-port] [src-channel] [sequence]",
Short: "Pay a fee to incentivize an existing IBC packet",
Long: strings.TrimSpace(`Pay a fee to incentivize an existing IBC packet.`),
Example: fmt.Sprintf("%s tx pay-packet-fee transfer channel-0 1 --recv-fee 10stake --ack-fee 10stake --timeout-fee 10stake", version.AppName),
Args: cobra.ExactArgs(3),
RunE: func(cmd *cobra.Command, args []string) error {
clientCtx, err := client.GetClientTxContext(cmd)
if err != nil {
return err
}

// NOTE: specifying non-nil relayers is currently unsupported
var relayers []string

sender := clientCtx.GetFromAddress().String()
seq, err := strconv.ParseUint(args[2], 10, 64)
if err != nil {
return err
}

packetID := channeltypes.NewPacketId(args[1], args[0], seq)

recvFeeStr, err := cmd.Flags().GetString(flagRecvFee)
if err != nil {
return err
}

recvFee, err := sdk.ParseCoinsNormalized(recvFeeStr)
if err != nil {
return err
}

ackFeeStr, err := cmd.Flags().GetString(flagAckFee)
if err != nil {
return err
}

ackFee, err := sdk.ParseCoinsNormalized(ackFeeStr)
if err != nil {
return err
}

timeoutFeeStr, err := cmd.Flags().GetString(flagTimeoutFee)
if err != nil {
return err
}

timeoutFee, err := sdk.ParseCoinsNormalized(timeoutFeeStr)
if err != nil {
return err
}

fee := types.Fee{
RecvFee: recvFee,
AckFee: ackFee,
TimeoutFee: timeoutFee,
}

identifiedPacketFee := types.NewIdentifiedPacketFee(packetID, fee, sender, relayers)

msg := types.NewMsgPayPacketFeeAsync(identifiedPacketFee)
return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msg)
},
}

cmd.Flags().String(flagRecvFee, "", "Fee paid to a relayer for relaying a packet receive.")
cmd.Flags().String(flagAckFee, "", "Fee paid to a relayer for relaying a packet acknowledgement.")
cmd.Flags().String(flagTimeoutFee, "", "Fee paid to a relayer for relaying a packet timeout.")
flags.AddTxFlagsToCmd(cmd)

return cmd
}
40 changes: 0 additions & 40 deletions modules/apps/29-fee/fee_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,9 @@ package fee_test
import (
"testing"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/stretchr/testify/suite"

"github.com/cosmos/ibc-go/v3/modules/apps/29-fee/types"
transfertypes "github.com/cosmos/ibc-go/v3/modules/apps/transfer/types"
clienttypes "github.com/cosmos/ibc-go/v3/modules/core/02-client/types"
channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types"
ibctesting "github.com/cosmos/ibc-go/v3/testing"
Expand All @@ -25,27 +23,11 @@ type FeeTestSuite struct {
path *ibctesting.Path
}

// TODO: remove and rename 'SetupMockTest' to 'SetupTest'
func (suite *FeeTestSuite) SetupTest() {
suite.coordinator = ibctesting.NewCoordinator(suite.T(), 2)
suite.chainA = suite.coordinator.GetChain(ibctesting.GetChainID(1))
suite.chainB = suite.coordinator.GetChain(ibctesting.GetChainID(2))

path := ibctesting.NewPath(suite.chainA, suite.chainB)
feeTransferVersion := string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: transfertypes.Version}))
path.EndpointA.ChannelConfig.Version = feeTransferVersion
path.EndpointB.ChannelConfig.Version = feeTransferVersion
path.EndpointA.ChannelConfig.PortID = transfertypes.PortID
path.EndpointB.ChannelConfig.PortID = transfertypes.PortID
suite.path = path
}

// TODO: rename to 'SetupTest' when the above function is removed
func (suite *FeeTestSuite) SetupMockTest() {
suite.coordinator = ibctesting.NewCoordinator(suite.T(), 2)
suite.chainA = suite.coordinator.GetChain(ibctesting.GetChainID(1))
suite.chainB = suite.coordinator.GetChain(ibctesting.GetChainID(2))

path := ibctesting.NewPath(suite.chainA, suite.chainB)
mockFeeVersion := string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: ibcmock.Version}))
path.EndpointA.ChannelConfig.Version = mockFeeVersion
Expand All @@ -59,28 +41,6 @@ func TestIBCFeeTestSuite(t *testing.T) {
suite.Run(t, new(FeeTestSuite))
}

// TODO: remove
func (suite *FeeTestSuite) CreateICS20Packet(coin sdk.Coin) channeltypes.Packet {

fungibleTokenPacket := transfertypes.NewFungibleTokenPacketData(
coin.Denom,
sdk.NewInt(100).String(),
suite.chainA.SenderAccount.GetAddress().String(),
suite.chainB.SenderAccount.GetAddress().String(),
)

return channeltypes.NewPacket(
fungibleTokenPacket.GetBytes(),
suite.chainA.SenderAccount.GetSequence(),
suite.path.EndpointA.ChannelConfig.PortID,
suite.path.EndpointA.ChannelID,
suite.path.EndpointB.ChannelConfig.PortID,
suite.path.EndpointB.ChannelID,
clienttypes.NewHeight(0, 100),
0,
)
}

func (suite *FeeTestSuite) CreateMockPacket() channeltypes.Packet {
return channeltypes.NewPacket(
ibcmock.MockPacketData,
Expand Down
15 changes: 8 additions & 7 deletions modules/apps/29-fee/ibc_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func (im IBCModule) OnChanOpenAck(
}

if versionMetadata.FeeVersion != types.Version {
return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected counterparty version: %s, got: %s", types.Version, versionMetadata.FeeVersion)
return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected counterparty fee version: %s, got: %s", types.Version, versionMetadata.FeeVersion)
}

// call underlying app's OnChanOpenAck callback with the counterparty app version.
Expand Down Expand Up @@ -196,15 +196,16 @@ func (im IBCModule) OnRecvPacket(

ack := im.app.OnRecvPacket(ctx, packet, relayer)

forwardRelayer, found := im.keeper.GetCounterpartyAddress(ctx, relayer.String())

// incase of async aknowledgement (ack == nil) store the ForwardRelayer address for use later
if ack == nil && found {
im.keeper.SetForwardRelayerAddress(ctx, channeltypes.NewPacketId(packet.GetSourceChannel(), packet.GetSourcePort(), packet.GetSequence()), forwardRelayer)
// incase of async aknowledgement (ack == nil) store the relayer address for use later during async WriteAcknowledgement
if ack == nil {
im.keeper.SetRelayerAddressForAsyncAck(ctx, channeltypes.NewPacketId(packet.GetSourceChannel(), packet.GetSourcePort(), packet.GetSequence()), relayer.String())
return nil
}

return types.NewIncentivizedAcknowledgement(forwardRelayer, ack.Acknowledgement())
// if forwardRelayer is not found we refund recv_fee
forwardRelayer, _ := im.keeper.GetCounterpartyAddress(ctx, relayer.String(), packet.GetSourceChannel())

return types.NewIncentivizedAcknowledgement(forwardRelayer, ack.Acknowledgement(), ack.Success())
}

// OnAcknowledgementPacket implements the IBCModule interface
Expand Down
51 changes: 32 additions & 19 deletions modules/apps/29-fee/ibc_module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
transfertypes "github.com/cosmos/ibc-go/v3/modules/apps/transfer/types"
channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types"
host "github.com/cosmos/ibc-go/v3/modules/core/24-host"
"github.com/cosmos/ibc-go/v3/modules/core/exported"
ibctesting "github.com/cosmos/ibc-go/v3/testing"
ibcmock "github.com/cosmos/ibc-go/v3/testing/mock"
)
Expand Down Expand Up @@ -59,7 +60,7 @@ func (suite *FeeTestSuite) TestOnChanOpenInit() {

suite.Run(tc.name, func() {
// reset suite
suite.SetupMockTest()
suite.SetupTest()
suite.coordinator.SetupConnections(suite.path)

// setup mock callback
Expand Down Expand Up @@ -150,7 +151,7 @@ func (suite *FeeTestSuite) TestOnChanOpenTry() {

suite.Run(tc.name, func() {
// reset suite
suite.SetupMockTest()
suite.SetupTest()
suite.coordinator.SetupConnections(suite.path)
suite.path.EndpointB.ChanOpenInit()

Expand Down Expand Up @@ -255,7 +256,7 @@ func (suite *FeeTestSuite) TestOnChanOpenAck() {
for _, tc := range testCases {
tc := tc
suite.Run(tc.name, func() {
suite.SetupMockTest()
suite.SetupTest()
suite.coordinator.SetupConnections(suite.path)

// setup mock callback
Expand Down Expand Up @@ -337,7 +338,7 @@ func (suite *FeeTestSuite) TestOnChanCloseInit() {
for _, tc := range testCases {
tc := tc
suite.Run(tc.name, func() {
suite.SetupMockTest()
suite.SetupTest()
suite.coordinator.Setup(suite.path) // setup channel

origBal := suite.chainA.GetSimApp().BankKeeper.GetAllBalances(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress())
Expand Down Expand Up @@ -415,7 +416,7 @@ func (suite *FeeTestSuite) TestOnChanCloseConfirm() {
for _, tc := range testCases {
tc := tc
suite.Run(tc.name, func() {
suite.SetupMockTest()
suite.SetupTest()
suite.coordinator.Setup(suite.path) // setup channel

origBal := suite.chainA.GetSimApp().BankKeeper.GetAllBalances(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress())
Expand Down Expand Up @@ -461,11 +462,18 @@ func (suite *FeeTestSuite) TestOnRecvPacket() {
true,
},
{
"source relayer is empty string",
"async write acknowledgement: ack is nil",
func() {
suite.chainB.GetSimApp().IBCFeeKeeper.SetCounterpartyAddress(suite.chainB.GetContext(), suite.chainA.SenderAccount.GetAddress().String(), "")
// setup mock callback
suite.chainB.GetSimApp().FeeMockModule.IBCApp.OnRecvPacket = func(
ctx sdk.Context,
packet channeltypes.Packet,
relayer sdk.AccAddress,
) exported.Acknowledgement {
return nil
}
},
false,
true,
true,
},
{
Expand All @@ -476,12 +484,20 @@ func (suite *FeeTestSuite) TestOnRecvPacket() {
true,
false,
},
{
"forward address is not found",
func() {
suite.chainB.GetSimApp().IBCFeeKeeper.SetCounterpartyAddress(suite.chainB.GetContext(), suite.chainA.SenderAccount.GetAddress().String(), "", suite.path.EndpointB.ChannelID)
},
false,
true,
},
}

for _, tc := range testCases {
tc := tc
suite.Run(tc.name, func() {
suite.SetupMockTest()
suite.SetupTest()
suite.coordinator.Setup(suite.path)

// set up a different channel to make sure that the test will error if the destination channel of the packet is not fee enabled
Expand All @@ -498,7 +514,7 @@ func (suite *FeeTestSuite) TestOnRecvPacket() {
cbs, ok := suite.chainB.App.GetIBCKeeper().Router.GetRoute(module)
suite.Require().True(ok)

suite.chainB.GetSimApp().IBCFeeKeeper.SetCounterpartyAddress(suite.chainB.GetContext(), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String())
suite.chainB.GetSimApp().IBCFeeKeeper.SetCounterpartyAddress(suite.chainB.GetContext(), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), suite.path.EndpointB.ChannelID)

// malleate test case
tc.malleate()
Expand All @@ -509,19 +525,16 @@ func (suite *FeeTestSuite) TestOnRecvPacket() {
case !tc.feeEnabled:
suite.Require().Equal(ibcmock.MockAcknowledgement, result)

case tc.forwardRelayer:
ack := types.IncentivizedAcknowledgement{
Result: ibcmock.MockAcknowledgement.Acknowledgement(),
ForwardRelayerAddress: suite.chainB.SenderAccount.GetAddress().String(),
}
suite.Require().Equal(ack, result)
case tc.forwardRelayer && result == nil:
suite.Require().Equal(nil, result)

case !tc.forwardRelayer:
ack := types.IncentivizedAcknowledgement{
expectedAck := types.IncentivizedAcknowledgement{
Result: ibcmock.MockAcknowledgement.Acknowledgement(),
ForwardRelayerAddress: "",
UnderlyingAppSuccess: true,
}
suite.Require().Equal(ack, result)
suite.Require().Equal(expectedAck, result)
}
})
}
Expand Down Expand Up @@ -723,7 +736,7 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() {
for _, tc := range testCases {
tc := tc
suite.Run(tc.name, func() {
suite.SetupMockTest()
suite.SetupTest()
suite.coordinator.Setup(suite.path)
packet := suite.CreateMockPacket()

Expand Down
Loading

0 comments on commit c4e6f41

Please sign in to comment.