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

chore: add ValidateBasic for MsgSendPacket #7468

Merged
merged 1 commit into from
Oct 15, 2024
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
1 change: 1 addition & 0 deletions modules/core/04-channel/v2/types/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,5 @@ var (
ErrInvalidPacket = errorsmod.Register(SubModuleName, 4, "invalid packet")
ErrInvalidPayload = errorsmod.Register(SubModuleName, 5, "invalid payload")
ErrSequenceSendNotFound = errorsmod.Register(SubModuleName, 6, "sequence send not found")
ErrInvalidPacketData = errorsmod.Register(SubModuleName, 7, "invalid packet data")
)
29 changes: 29 additions & 0 deletions modules/core/04-channel/v2/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"

clienttypes "github.com/cosmos/ibc-go/v9/modules/core/02-client/types"
channeltypesv1 "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

feels weird mixing these personally? Would prefer if we just moved over all errors (i.e dunno what the error code for this error is? it might conflict with one we define in errors.go for chan/v2)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were discussing this with @chatton and thought about importing for now and then having a cleanup issue about moving everything to v2. Though I'm with doing it now if we prefer

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perfectly fine with me as long as there's an issue to track it 💪

commitmenttypes "github.com/cosmos/ibc-go/v9/modules/core/23-commitment/types/v2"
host "github.com/cosmos/ibc-go/v9/modules/core/24-host"
ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors"
Expand Down Expand Up @@ -81,6 +82,34 @@ func NewMsgSendPacket(sourceChannel string, timeoutTimestamp uint64, signer stri
}
}

// ValidateBasic performs basic checks on a MsgSendPacket.
func (msg *MsgSendPacket) ValidateBasic() error {
if err := host.ChannelIdentifierValidator(msg.SourceChannel); err != nil {
return err
}

if msg.TimeoutTimestamp == 0 {
return errorsmod.Wrap(channeltypesv1.ErrInvalidTimeout, "timeout must not be 0")
}

if len(msg.PacketData) != 1 {
return errorsmod.Wrapf(ErrInvalidPacketData, "packet data must be of length 1, got %d instead", len(msg.PacketData))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

partially also addreses #7390 (can close it after validation is done for all msgs)

}

for _, pd := range msg.PacketData {
if err := pd.ValidateBasic(); err != nil {
return err
}
}

_, err := sdk.AccAddressFromBech32(msg.Signer)
if err != nil {
return errorsmod.Wrapf(ibcerrors.ErrInvalidAddress, "string could not be parsed as address: %v", err)
}

return nil
}

// NewMsgRecvPacket creates a new MsgRecvPacket instance.
func NewMsgRecvPacket(packet Packet, proofCommitment []byte, proofHeight clienttypes.Height, signer string) *MsgRecvPacket {
return &MsgRecvPacket{
Expand Down
69 changes: 69 additions & 0 deletions modules/core/04-channel/v2/types/msgs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/stretchr/testify/suite"

channeltypesv1 "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types"
"github.com/cosmos/ibc-go/v9/modules/core/04-channel/v2/types"
commitmenttypes "github.com/cosmos/ibc-go/v9/modules/core/23-commitment/types"
host "github.com/cosmos/ibc-go/v9/modules/core/24-host"
Expand Down Expand Up @@ -142,3 +143,71 @@ func (s *TypesTestSuite) TestMsgCreateChannelValidateBasic() {
}
}
}

func (s *TypesTestSuite) TestMsgSendPacketValidateBasic() {
var msg *types.MsgSendPacket
testCases := []struct {
name string
malleate func()
expError error
}{
{
name: "success",
malleate: func() {},
},
{
name: "failure: invalid source channel",
malleate: func() {
msg.SourceChannel = ""
},
expError: host.ErrInvalidID,
},
{
name: "failure: invalid timestamp",
malleate: func() {
msg.TimeoutTimestamp = 0
},
expError: channeltypesv1.ErrInvalidTimeout,
},
{
name: "failure: invalid length for packetdata",
malleate: func() {
msg.PacketData = []types.PacketData{}
},
expError: types.ErrInvalidPacketData,
},
{
name: "failure: invalid packetdata",
malleate: func() {
msg.PacketData[0].DestinationPort = ""
},
expError: host.ErrInvalidID,
},
{
name: "failure: invalid signer",
malleate: func() {
msg.Signer = ""
},
expError: ibcerrors.ErrInvalidAddress,
},
}
for _, tc := range testCases {
s.Run(tc.name, func() {
msg = types.NewMsgSendPacket(
ibctesting.FirstChannelID, s.chainA.GetTimeoutTimestamp(),
s.chainA.SenderAccount.GetAddress().String(),
types.PacketData{SourcePort: ibctesting.MockPort, DestinationPort: ibctesting.MockPort, Payload: types.NewPayload("ics20-1", "json", ibctesting.MockPacketData)},
)

tc.malleate()

err := msg.ValidateBasic()
expPass := tc.expError == nil
if expPass {
s.Require().NoError(err)
} else {
ibctesting.RequireErrorIsOrContains(s.T(), err, tc.expError)
}
})
}
}
Loading