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

Modify MsgSendTx fields based on audit (backport #2280) #2427

Merged
merged 1 commit into from
Sep 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 1 addition & 2 deletions docs/ibc/proto-docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -1656,9 +1656,8 @@ MsgSendTx defines the payload for Msg/SendTx
| ----- | ---- | ----- | ----------- |
| `owner` | [string](#string) | | |
| `connection_id` | [string](#string) | | |
| `timeout_height` | [ibc.core.client.v1.Height](#ibc.core.client.v1.Height) | | Timeout height relative to the current block height. The timeout is disabled when set to 0. |
| `timeout_timestamp` | [uint64](#uint64) | | Timeout timestamp in absolute nanoseconds since unix epoch. The timeout is disabled when set to 0. |
| `packet_data` | [ibc.applications.interchain_accounts.v1.InterchainAccountPacketData](#ibc.applications.interchain_accounts.v1.InterchainAccountPacketData) | | |
| `relative_timeout` | [uint64](#uint64) | | Relative timeout timestamp provided will be added to the current block time during transaction execution. The timeout timestamp must be non-zero. |



Expand Down
25 changes: 6 additions & 19 deletions modules/apps/27-interchain-accounts/controller/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,12 @@ import (

"github.com/cosmos/ibc-go/v6/modules/apps/27-interchain-accounts/controller/types"
icatypes "github.com/cosmos/ibc-go/v6/modules/apps/27-interchain-accounts/types"
clienttypes "github.com/cosmos/ibc-go/v6/modules/core/02-client/types"
)

const (
// The controller chain channel version
flagVersion = "version"
flagPacketTimeoutHeight = "packet-timeout-height"
flagPacketTimeoutTimestamp = "packet-timeout-timestamp"
flagVersion = "version"
flagRelativePacketTimeout = "relative-packet-timeout"
)

func newRegisterInterchainAccountCmd() *cobra.Command {
Expand Down Expand Up @@ -65,8 +63,7 @@ func newSendTxCmd() *cobra.Command {
Short: "Send an interchain account tx on the provided connection.",
Long: strings.TrimSpace(`Submits pre-built packet data containing messages to be executed on the host chain
and attempts to send the packet. Packet data is provided as json, file or string. An
appropriate relative timeoutTimestamp must be provided with flag {packet-timeout-timestamp}, along with a timeoutHeight
via {packet-timeout-timestamp}`),
appropriate relative timeoutTimestamp must be provided with flag {relative-packet-timeout}`),
Args: cobra.ExactArgs(2),
RunE: func(cmd *cobra.Command, args []string) error {
clientCtx, err := client.GetClientTxContext(cmd)
Expand Down Expand Up @@ -94,28 +91,18 @@ via {packet-timeout-timestamp}`),
}
}

timeoutHeightStr, err := cmd.Flags().GetString(flagPacketTimeoutHeight)
if err != nil {
return err
}
timeoutHeight, err := clienttypes.ParseHeight(timeoutHeightStr)
if err != nil {
return err
}

timeoutTimestamp, err := cmd.Flags().GetUint64(flagPacketTimeoutTimestamp)
relativeTimeoutTimestamp, err := cmd.Flags().GetUint64(flagRelativePacketTimeout)
if err != nil {
return err
}

msg := types.NewMsgSendTx(owner, connectionID, timeoutHeight, timeoutTimestamp, icaMsgData)
msg := types.NewMsgSendTx(owner, connectionID, relativeTimeoutTimestamp, icaMsgData)

return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msg)
},
}

cmd.Flags().String(flagPacketTimeoutHeight, icatypes.DefaultRelativePacketTimeoutHeight, "Packet timeout block height. The timeout is disabled when set to 0-0.")
cmd.Flags().Uint64(flagPacketTimeoutTimestamp, icatypes.DefaultRelativePacketTimeoutTimestamp, "Packet timeout timestamp in nanoseconds from now. Default is 10 minutes. The timeout is disabled when set to 0.")
cmd.Flags().Uint64(flagRelativePacketTimeout, icatypes.DefaultRelativePacketTimeoutTimestamp, "Relative packet timeout in nanoseconds from now. Default is 10 minutes.")
flags.AddTxFlagsToCmd(cmd)

return cmd
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func (s msgServer) SendTx(goCtx context.Context, msg *types.MsgSendTx) (*types.M
}

// explicitly passing nil as the argument is discarded as the channel capability is retrieved in SendTx.
absoluteTimeout := uint64(ctx.BlockTime().UnixNano()) + msg.TimeoutTimestamp
absoluteTimeout := uint64(ctx.BlockTime().UnixNano()) + msg.RelativeTimeout
seq, err := s.Keeper.SendTx(ctx, nil, msg.ConnectionId, portID, msg.PacketData, absoluteTimeout)
if err != nil {
return nil, err
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"github.com/cosmos/ibc-go/v6/modules/apps/27-interchain-accounts/controller/keeper"
"github.com/cosmos/ibc-go/v6/modules/apps/27-interchain-accounts/controller/types"
icatypes "github.com/cosmos/ibc-go/v6/modules/apps/27-interchain-accounts/types"
clienttypes "github.com/cosmos/ibc-go/v6/modules/core/02-client/types"
channeltypes "github.com/cosmos/ibc-go/v6/modules/core/04-channel/types"
host "github.com/cosmos/ibc-go/v6/modules/core/24-host"
ibctesting "github.com/cosmos/ibc-go/v6/testing"
Expand Down Expand Up @@ -182,7 +181,7 @@ func (suite *KeeperTestSuite) TestSubmitTx() {
timeoutTimestamp := uint64(suite.chainA.GetContext().BlockTime().Add(time.Minute).UnixNano())
connectionID := path.EndpointA.ConnectionID

msg = types.NewMsgSendTx(owner, connectionID, clienttypes.ZeroHeight(), timeoutTimestamp, packetData)
msg = types.NewMsgSendTx(owner, connectionID, timeoutTimestamp, packetData)

tc.malleate() // malleate mutates test data

Expand Down
21 changes: 9 additions & 12 deletions modules/apps/27-interchain-accounts/controller/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ import (
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"

icatypes "github.com/cosmos/ibc-go/v6/modules/apps/27-interchain-accounts/types"
clienttypes "github.com/cosmos/ibc-go/v6/modules/core/02-client/types"
channelerrors "github.com/cosmos/ibc-go/v6/modules/core/04-channel/types"
host "github.com/cosmos/ibc-go/v6/modules/core/24-host"
)

Expand Down Expand Up @@ -51,13 +49,12 @@ func (msg MsgRegisterInterchainAccount) GetSigners() []sdk.AccAddress {
}

// NewMsgSendTx creates a new instance of MsgSendTx
func NewMsgSendTx(owner, connectionID string, timeoutHeight clienttypes.Height, timeoutTimestamp uint64, packetData icatypes.InterchainAccountPacketData) *MsgSendTx {
func NewMsgSendTx(owner, connectionID string, relativeTimeoutTimestamp uint64, packetData icatypes.InterchainAccountPacketData) *MsgSendTx {
return &MsgSendTx{
ConnectionId: connectionID,
Owner: owner,
TimeoutHeight: timeoutHeight,
TimeoutTimestamp: timeoutTimestamp,
PacketData: packetData,
ConnectionId: connectionID,
Owner: owner,
RelativeTimeout: relativeTimeoutTimestamp,
PacketData: packetData,
}
}

Expand All @@ -75,14 +72,14 @@ func (msg MsgSendTx) ValidateBasic() error {
return sdkerrors.Wrapf(sdkerrors.ErrInvalidAddress, "failed to parse owner address: %s", msg.Owner)
}

if msg.TimeoutHeight.IsZero() && msg.TimeoutTimestamp == 0 {
return sdkerrors.Wrap(channelerrors.ErrInvalidTimeout, "msg timeout height and msg timeout timestamp cannot both be 0")
}

if err := msg.PacketData.ValidateBasic(); err != nil {
return sdkerrors.Wrap(err, "invalid interchain account packet data")
}

if msg.RelativeTimeout == 0 {
return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "relative timeout cannot be zero")
}

return nil
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"github.com/cosmos/ibc-go/v6/modules/apps/27-interchain-accounts/controller/types"
icatypes "github.com/cosmos/ibc-go/v6/modules/apps/27-interchain-accounts/types"
feetypes "github.com/cosmos/ibc-go/v6/modules/apps/29-fee/types"
clienttypes "github.com/cosmos/ibc-go/v6/modules/core/02-client/types"
ibctesting "github.com/cosmos/ibc-go/v6/testing"
"github.com/cosmos/ibc-go/v6/testing/simapp"
)
Expand Down Expand Up @@ -133,9 +132,9 @@ func TestMsgSendTxValidateBasic(t *testing.T) {
false,
},
{
"timeout height and timestamp are both not set",
"relative timeout is not set",
func() {
msg.TimeoutTimestamp = 0
msg.RelativeTimeout = 0
},
false,
},
Expand Down Expand Up @@ -167,7 +166,6 @@ func TestMsgSendTxValidateBasic(t *testing.T) {
msg = types.NewMsgSendTx(
ibctesting.TestAccAddress,
ibctesting.FirstConnectionID,
clienttypes.ZeroHeight(),
100000,
packetData,
)
Expand Down Expand Up @@ -204,7 +202,6 @@ func TestMsgSendTxGetSigners(t *testing.T) {
msg := types.NewMsgSendTx(
ibctesting.TestAccAddress,
ibctesting.FirstConnectionID,
clienttypes.ZeroHeight(),
100000,
packetData,
)
Expand Down
Loading