Skip to content

Commit

Permalink
Modify MsgSendTx fields based on audit (#2280)
Browse files Browse the repository at this point in the history
(cherry picked from commit 3c21b5e)
  • Loading branch information
chatton authored and mergify[bot] committed Sep 28, 2022
1 parent 32bf4d0 commit e86a15e
Show file tree
Hide file tree
Showing 8 changed files with 80 additions and 158 deletions.
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

0 comments on commit e86a15e

Please sign in to comment.