Skip to content

Commit

Permalink
chore: update MsgPayPacketFeeAsync fields (#979)
Browse files Browse the repository at this point in the history
* adding new proto types and codegen

* refactoring ics29 fees for more efficient storage

* updating tests

* fixing typo in protodoc comments

* updating protos and codegen

* updating MsgPayPacketFeeAsync handler and tests
  • Loading branch information
damiannolan authored Feb 24, 2022
1 parent 15fa37b commit 9350d53
Show file tree
Hide file tree
Showing 8 changed files with 140 additions and 92 deletions.
3 changes: 2 additions & 1 deletion docs/ibc/proto-docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -1041,7 +1041,8 @@ This Msg can be used to pay for a packet at a specified sequence (instead of the

| Field | Type | Label | Description |
| ----- | ---- | ----- | ----------- |
| `identified_packet_fee` | [IdentifiedPacketFee](#ibc.applications.fee.v1.IdentifiedPacketFee) | | identified packet to pay fee for identified fee must contain the refund address which is also signer of this message |
| `packet_id` | [ibc.core.channel.v1.PacketId](#ibc.core.channel.v1.PacketId) | | unique packet identifier |
| `packet_fee` | [PacketFee](#ibc.applications.fee.v1.PacketFee) | | packet fee for incentivization |



Expand Down
4 changes: 2 additions & 2 deletions modules/apps/29-fee/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,9 @@ func NewPayPacketFeeAsyncTxCmd() *cobra.Command {
TimeoutFee: timeoutFee,
}

identifiedPacketFee := types.NewIdentifiedPacketFee(packetID, fee, sender, relayers)
packetFee := types.NewPacketFee(fee, sender, relayers)
msg := types.NewMsgPayPacketFeeAsync(packetID, packetFee)

msg := types.NewMsgPayPacketFeeAsync(identifiedPacketFee)
return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msg)
},
}
Expand Down
4 changes: 1 addition & 3 deletions modules/apps/29-fee/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,7 @@ func (k Keeper) PayPacketFee(goCtx context.Context, msg *types.MsgPayPacketFee)
func (k Keeper) PayPacketFeeAsync(goCtx context.Context, msg *types.MsgPayPacketFeeAsync) (*types.MsgPayPacketFeeAsyncResponse, error) {
ctx := sdk.UnwrapSDKContext(goCtx)

// TODO: Update MsgPayPacketFeeAsync to include PacketFee in favour of IdentifiedPacketFee
packetFee := types.NewPacketFee(msg.IdentifiedPacketFee.Fee, msg.IdentifiedPacketFee.RefundAddress, msg.IdentifiedPacketFee.Relayers)
if err := k.EscrowPacketFee(ctx, msg.IdentifiedPacketFee.PacketId, packetFee); err != nil {
if err := k.EscrowPacketFee(ctx, msg.PacketId, msg.PacketFee); err != nil {
return nil, err
}

Expand Down
6 changes: 3 additions & 3 deletions modules/apps/29-fee/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,12 +119,12 @@ func (suite *KeeperTestSuite) TestPayPacketFeeAsync() {
seq, _ := suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetNextSequenceSend(ctxA, suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID)

// build fee
packetId := channeltypes.NewPacketId(channelID, suite.path.EndpointA.ChannelConfig.PortID, seq)
identifiedPacketFee := types.IdentifiedPacketFee{PacketId: packetId, Fee: fee, RefundAddress: refundAcc.String(), Relayers: []string{}}
packetID := channeltypes.NewPacketId(channelID, suite.path.EndpointA.ChannelConfig.PortID, seq)
packetFee := types.NewPacketFee(fee, refundAcc.String(), nil)

tc.malleate()

msg := types.NewMsgPayPacketFeeAsync(identifiedPacketFee)
msg := types.NewMsgPayPacketFeeAsync(packetID, packetFee)
_, err := suite.chainA.SendMsgs(msg)

if tc.expPass {
Expand Down
17 changes: 8 additions & 9 deletions modules/apps/29-fee/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,22 +118,21 @@ func (msg MsgPayPacketFee) GetSignBytes() []byte {
}

// NewMsgPayPacketAsync creates a new instance of MsgPayPacketFee
func NewMsgPayPacketFeeAsync(identifiedPacketFee IdentifiedPacketFee) *MsgPayPacketFeeAsync {
func NewMsgPayPacketFeeAsync(packetID channeltypes.PacketId, packetFee PacketFee) *MsgPayPacketFeeAsync {
return &MsgPayPacketFeeAsync{
IdentifiedPacketFee: identifiedPacketFee,
PacketId: packetID,
PacketFee: packetFee,
}
}

// ValidateBasic performs a basic check of the MsgPayPacketFeeAsync fields
func (msg MsgPayPacketFeeAsync) ValidateBasic() error {
// signer check
_, err := sdk.AccAddressFromBech32(msg.IdentifiedPacketFee.RefundAddress)
if err != nil {
return sdkerrors.Wrap(err, "failed to convert msg.Signer into sdk.AccAddress")
if err := msg.PacketId.Validate(); err != nil {
return err
}

if err = msg.IdentifiedPacketFee.Validate(); err != nil {
return sdkerrors.Wrap(err, "Invalid IdentifiedPacketFee")
if err := msg.PacketFee.Validate(); err != nil {
return err
}

return nil
Expand All @@ -142,7 +141,7 @@ func (msg MsgPayPacketFeeAsync) ValidateBasic() error {
// GetSigners implements sdk.Msg
// The signer of the fee message must be the refund address
func (msg MsgPayPacketFeeAsync) GetSigners() []sdk.AccAddress {
signer, err := sdk.AccAddressFromBech32(msg.IdentifiedPacketFee.RefundAddress)
signer, err := sdk.AccAddressFromBech32(msg.PacketFee.RefundAddress)
if err != nil {
panic(err)
}
Expand Down
47 changes: 23 additions & 24 deletions modules/apps/29-fee/types/msgs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -319,9 +319,10 @@ func TestMsgPayPacketFeeAsyncValidation(t *testing.T) {
tc.malleate()
fee = Fee{receiveFee, ackFee, timeoutFee}

packetId := channeltypes.NewPacketId(channelID, portID, seq)
identifiedPacketFee := IdentifiedPacketFee{PacketId: packetId, Fee: fee, RefundAddress: signer, Relayers: relayers}
msg := NewMsgPayPacketFeeAsync(identifiedPacketFee)
packetID := channeltypes.NewPacketId(channelID, portID, seq)
packetFee := NewPacketFee(fee, signer, relayers)

msg := NewMsgPayPacketFeeAsync(packetID, packetFee)

err := msg.ValidateBasic()

Expand All @@ -335,20 +336,15 @@ func TestMsgPayPacketFeeAsyncValidation(t *testing.T) {

// TestRegisterCounterpartyAddressGetSigners tests GetSigners
func TestPayPacketFeeAsyncGetSigners(t *testing.T) {
addr := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address())
// build message
channelID := validChannelID
portID := validPortID
fee := Fee{validCoins, validCoins, validCoins}
seq := uint64(1)
packetId := channeltypes.NewPacketId(channelID, portID, seq)
identifiedPacketFee := IdentifiedPacketFee{PacketId: packetId, Fee: fee, RefundAddress: addr.String(), Relayers: nil}
msg := NewMsgPayPacketFeeAsync(identifiedPacketFee)
refundAddr := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address())
fee := NewFee(validCoins, validCoins, validCoins)

// GetSigners
res := msg.GetSigners()
packetID := channeltypes.NewPacketId(validChannelID, validPortID, 1)
packetFee := NewPacketFee(fee, refundAddr.String(), nil)

require.Equal(t, []sdk.AccAddress{addr}, res)
msg := NewMsgPayPacketFeeAsync(packetID, packetFee)

require.Equal(t, []sdk.AccAddress{refundAddr}, msg.GetSigners())
}

// TestMsgPayPacketFeeAsyncRoute tests Route for MsgPayPacketFeeAsync
Expand All @@ -360,9 +356,10 @@ func TestMsgPayPacketFeeAsyncRoute(t *testing.T) {
portID := validPortID
fee := Fee{validCoins, validCoins, validCoins}
seq := uint64(1)
packetId := channeltypes.NewPacketId(channelID, portID, seq)
identifiedPacketFee := IdentifiedPacketFee{PacketId: packetId, Fee: fee, RefundAddress: addr.String(), Relayers: nil}
msg := NewMsgPayPacketFeeAsync(identifiedPacketFee)
packetID := channeltypes.NewPacketId(channelID, portID, seq)
packetFee := NewPacketFee(fee, addr.String(), nil)

msg := NewMsgPayPacketFeeAsync(packetID, packetFee)

require.Equal(t, RouterKey, msg.Route())
}
Expand All @@ -376,9 +373,10 @@ func TestMsgPayPacketFeeAsyncType(t *testing.T) {
portID := validPortID
fee := Fee{validCoins, validCoins, validCoins}
seq := uint64(1)
packetId := channeltypes.NewPacketId(channelID, portID, seq)
identifiedPacketFee := IdentifiedPacketFee{PacketId: packetId, Fee: fee, RefundAddress: addr.String(), Relayers: nil}
msg := NewMsgPayPacketFeeAsync(identifiedPacketFee)
packetID := channeltypes.NewPacketId(channelID, portID, seq)
packetFee := NewPacketFee(fee, addr.String(), nil)

msg := NewMsgPayPacketFeeAsync(packetID, packetFee)

require.Equal(t, "payPacketFeeAsync", msg.Type())
}
Expand All @@ -392,9 +390,10 @@ func TestMsgPayPacketFeeAsyncGetSignBytes(t *testing.T) {
portID := validPortID
fee := Fee{validCoins, validCoins, validCoins}
seq := uint64(1)
packetId := channeltypes.NewPacketId(channelID, portID, seq)
identifiedPacketFee := IdentifiedPacketFee{PacketId: packetId, Fee: fee, RefundAddress: addr.String(), Relayers: nil}
msg := NewMsgPayPacketFeeAsync(identifiedPacketFee)
packetID := channeltypes.NewPacketId(channelID, portID, seq)
packetFee := NewPacketFee(fee, addr.String(), nil)

msg := NewMsgPayPacketFeeAsync(packetID, packetFee)

require.NotPanics(t, func() {
_ = msg.GetSignBytes()
Expand Down
141 changes: 95 additions & 46 deletions modules/apps/29-fee/types/tx.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 6 additions & 4 deletions proto/ibc/applications/fee/v1/tx.proto
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ option go_package = "github.com/cosmos/ibc-go/v3/modules/apps/29-fee/types";

import "gogoproto/gogo.proto";
import "ibc/applications/fee/v1/fee.proto";
import "ibc/core/channel/v1/channel.proto";

// Msg defines the ibc/fee Msg service.
service Msg {
Expand Down Expand Up @@ -64,10 +65,11 @@ message MsgPayPacketFeeAsync {
option (gogoproto.equal) = false;
option (gogoproto.goproto_getters) = false;

// identified packet to pay fee for
// identified fee must contain the refund address which is also signer of this message
ibc.applications.fee.v1.IdentifiedPacketFee identified_packet_fee = 1
[(gogoproto.moretags) = "yaml:\"identified_packet_fee\"", (gogoproto.nullable) = false];
// unique packet identifier
ibc.core.channel.v1.PacketId packet_id = 1
[(gogoproto.moretags) = "yaml:\"packet_id\"", (gogoproto.nullable) = false];
// packet fee for incentivization
PacketFee packet_fee = 2 [(gogoproto.moretags) = "yaml:\"packet_fee\"", (gogoproto.nullable) = false];
}

// MsgPayPacketFeeAsyncResponse defines the response type for Msg/PayPacketFeeAsync
Expand Down

0 comments on commit 9350d53

Please sign in to comment.