Skip to content

Commit

Permalink
Remove ProofHeight and Proof Validation in msg.ValidateBasic (#3061)
Browse files Browse the repository at this point in the history
  • Loading branch information
@charleenfei
charleenfei authored Feb 2, 2023
1 parent 72fcd65 commit 3715153
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 39 deletions.
28 changes: 0 additions & 28 deletions modules/core/04-channel/types/msgs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"

clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
commitmenttypes "github.com/cosmos/ibc-go/v7/modules/core/23-commitment/types"
host "github.com/cosmos/ibc-go/v7/modules/core/24-host"
)

Expand Down Expand Up @@ -92,9 +91,6 @@ func (msg MsgChannelOpenTry) ValidateBasic() error {
if msg.PreviousChannelId != "" {
return sdkerrors.Wrap(ErrInvalidChannelIdentifier, "previous channel identifier must be empty, this field has been deprecated as crossing hellos are no longer supported")
}
if len(msg.ProofInit) == 0 {
return sdkerrors.Wrap(commitmenttypes.ErrInvalidProof, "cannot submit an empty proof init")
}
if msg.Channel.State != TRYOPEN {
return sdkerrors.Wrapf(ErrInvalidChannelState,
"channel state must be TRYOPEN in MsgChannelOpenTry. expected: %s, got: %s",
Expand Down Expand Up @@ -153,9 +149,6 @@ func (msg MsgChannelOpenAck) ValidateBasic() error {
if err := host.ChannelIdentifierValidator(msg.CounterpartyChannelId); err != nil {
return sdkerrors.Wrap(err, "invalid counterparty channel ID")
}
if len(msg.ProofTry) == 0 {
return sdkerrors.Wrap(commitmenttypes.ErrInvalidProof, "cannot submit an empty proof try")
}
_, err := sdk.AccAddressFromBech32(msg.Signer)
if err != nil {
return sdkerrors.Wrapf(sdkerrors.ErrInvalidAddress, "string could not be parsed as address: %v", err)
Expand Down Expand Up @@ -198,9 +191,6 @@ func (msg MsgChannelOpenConfirm) ValidateBasic() error {
if !IsValidChannelID(msg.ChannelId) {
return ErrInvalidChannelIdentifier
}
if len(msg.ProofAck) == 0 {
return sdkerrors.Wrap(commitmenttypes.ErrInvalidProof, "cannot submit an empty proof ack")
}
_, err := sdk.AccAddressFromBech32(msg.Signer)
if err != nil {
return sdkerrors.Wrapf(sdkerrors.ErrInvalidAddress, "string could not be parsed as address: %v", err)
Expand Down Expand Up @@ -282,9 +272,6 @@ func (msg MsgChannelCloseConfirm) ValidateBasic() error {
if !IsValidChannelID(msg.ChannelId) {
return ErrInvalidChannelIdentifier
}
if len(msg.ProofInit) == 0 {
return sdkerrors.Wrap(commitmenttypes.ErrInvalidProof, "cannot submit an empty proof init")
}
_, err := sdk.AccAddressFromBech32(msg.Signer)
if err != nil {
return sdkerrors.Wrapf(sdkerrors.ErrInvalidAddress, "string could not be parsed as address: %v", err)
Expand Down Expand Up @@ -320,9 +307,6 @@ func NewMsgRecvPacket(

// ValidateBasic implements sdk.Msg
func (msg MsgRecvPacket) ValidateBasic() error {
if len(msg.ProofCommitment) == 0 {
return sdkerrors.Wrap(commitmenttypes.ErrInvalidProof, "cannot submit an empty proof")
}
_, err := sdk.AccAddressFromBech32(msg.Signer)
if err != nil {
return sdkerrors.Wrapf(sdkerrors.ErrInvalidAddress, "string could not be parsed as address: %v", err)
Expand Down Expand Up @@ -366,9 +350,6 @@ func NewMsgTimeout(

// ValidateBasic implements sdk.Msg
func (msg MsgTimeout) ValidateBasic() error {
if len(msg.ProofUnreceived) == 0 {
return sdkerrors.Wrap(commitmenttypes.ErrInvalidProof, "cannot submit an empty unreceived proof")
}
if msg.NextSequenceRecv == 0 {
return sdkerrors.Wrap(sdkerrors.ErrInvalidSequence, "next sequence receive cannot be 0")
}
Expand Down Expand Up @@ -411,12 +392,6 @@ func (msg MsgTimeoutOnClose) ValidateBasic() error {
if msg.NextSequenceRecv == 0 {
return sdkerrors.Wrap(sdkerrors.ErrInvalidSequence, "next sequence receive cannot be 0")
}
if len(msg.ProofUnreceived) == 0 {
return sdkerrors.Wrap(commitmenttypes.ErrInvalidProof, "cannot submit an empty proof")
}
if len(msg.ProofClose) == 0 {
return sdkerrors.Wrap(commitmenttypes.ErrInvalidProof, "cannot submit an empty proof of closed counterparty channel end")
}
_, err := sdk.AccAddressFromBech32(msg.Signer)
if err != nil {
return sdkerrors.Wrapf(sdkerrors.ErrInvalidAddress, "string could not be parsed as address: %v", err)
Expand Down Expand Up @@ -455,9 +430,6 @@ func NewMsgAcknowledgement(

// ValidateBasic implements sdk.Msg
func (msg MsgAcknowledgement) ValidateBasic() error {
if len(msg.ProofAcked) == 0 {
return sdkerrors.Wrap(commitmenttypes.ErrInvalidProof, "cannot submit an empty proof")
}
if len(msg.Acknowledgement) == 0 {
return sdkerrors.Wrap(ErrInvalidAcknowledgement, "ack bytes cannot be empty")
}
Expand Down
11 changes: 0 additions & 11 deletions modules/core/04-channel/types/msgs_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,6 @@ var (
packet = types.NewPacket(validPacketData, 1, portid, chanid, cpportid, cpchanid, timeoutHeight, timeoutTimestamp)
invalidPacket = types.NewPacket(unknownPacketData, 0, portid, chanid, cpportid, cpchanid, timeoutHeight, timeoutTimestamp)

emptyProof = []byte{}

addr = sdk.AccAddress("testaddr111111111111").String()
emptyAddr string

Expand Down Expand Up @@ -164,7 +162,6 @@ func (suite *TypesTestSuite) TestMsgChannelOpenTryValidateBasic() {
{"", types.NewMsgChannelOpenTry(portid, "", types.UNORDERED, connHops, cpportid, cpchanid, version, suite.proof, height, addr), true},
{"invalid counterparty port id", types.NewMsgChannelOpenTry(portid, version, types.UNORDERED, connHops, invalidPort, cpchanid, version, suite.proof, height, addr), false},
{"invalid counterparty channel id", types.NewMsgChannelOpenTry(portid, version, types.UNORDERED, connHops, cpportid, invalidChannel, version, suite.proof, height, addr), false},
{"empty proof", types.NewMsgChannelOpenTry(portid, version, types.UNORDERED, connHops, cpportid, cpchanid, version, emptyProof, height, addr), false},
{"channel not in TRYOPEN state", &types.MsgChannelOpenTry{portid, "", initChannel, version, suite.proof, height, addr}, false},
{"previous channel id is not empty", &types.MsgChannelOpenTry{portid, chanid, initChannel, version, suite.proof, height, addr}, false},
}
Expand Down Expand Up @@ -198,7 +195,6 @@ func (suite *TypesTestSuite) TestMsgChannelOpenAckValidateBasic() {
{"too long channel id", types.NewMsgChannelOpenAck(portid, invalidLongChannel, chanid, version, suite.proof, height, addr), false},
{"channel id contains non-alpha", types.NewMsgChannelOpenAck(portid, invalidChannel, chanid, version, suite.proof, height, addr), false},
{"", types.NewMsgChannelOpenAck(portid, chanid, chanid, "", suite.proof, height, addr), true},
{"empty proof", types.NewMsgChannelOpenAck(portid, chanid, chanid, version, emptyProof, height, addr), false},
{"invalid counterparty channel id", types.NewMsgChannelOpenAck(portid, chanid, invalidShortChannel, version, suite.proof, height, addr), false},
}

Expand Down Expand Up @@ -230,7 +226,6 @@ func (suite *TypesTestSuite) TestMsgChannelOpenConfirmValidateBasic() {
{"too short channel id", types.NewMsgChannelOpenConfirm(portid, invalidShortChannel, suite.proof, height, addr), false},
{"too long channel id", types.NewMsgChannelOpenConfirm(portid, invalidLongChannel, suite.proof, height, addr), false},
{"channel id contains non-alpha", types.NewMsgChannelOpenConfirm(portid, invalidChannel, suite.proof, height, addr), false},
{"empty proof", types.NewMsgChannelOpenConfirm(portid, chanid, emptyProof, height, addr), false},
}

for _, tc := range testCases {
Expand Down Expand Up @@ -291,7 +286,6 @@ func (suite *TypesTestSuite) TestMsgChannelCloseConfirmValidateBasic() {
{"too short channel id", types.NewMsgChannelCloseConfirm(portid, invalidShortChannel, suite.proof, height, addr), false},
{"too long channel id", types.NewMsgChannelCloseConfirm(portid, invalidLongChannel, suite.proof, height, addr), false},
{"channel id contains non-alpha", types.NewMsgChannelCloseConfirm(portid, invalidChannel, suite.proof, height, addr), false},
{"empty proof", types.NewMsgChannelCloseConfirm(portid, chanid, emptyProof, height, addr), false},
}

for _, tc := range testCases {
Expand All @@ -316,7 +310,6 @@ func (suite *TypesTestSuite) TestMsgRecvPacketValidateBasic() {
expPass bool
}{
{"success", types.NewMsgRecvPacket(packet, suite.proof, height, addr), true},
{"proof contain empty proof", types.NewMsgRecvPacket(packet, emptyProof, height, addr), false},
{"missing signer address", types.NewMsgRecvPacket(packet, suite.proof, height, emptyAddr), false},
{"invalid packet", types.NewMsgRecvPacket(invalidPacket, suite.proof, height, addr), false},
}
Expand Down Expand Up @@ -353,7 +346,6 @@ func (suite *TypesTestSuite) TestMsgTimeoutValidateBasic() {
{"success", types.NewMsgTimeout(packet, 1, suite.proof, height, addr), true},
{"seq 0", types.NewMsgTimeout(packet, 0, suite.proof, height, addr), false},
{"missing signer address", types.NewMsgTimeout(packet, 1, suite.proof, height, emptyAddr), false},
{"cannot submit an empty proof", types.NewMsgTimeout(packet, 1, emptyProof, height, addr), false},
{"invalid packet", types.NewMsgTimeout(invalidPacket, 1, suite.proof, height, addr), false},
}

Expand All @@ -380,8 +372,6 @@ func (suite *TypesTestSuite) TestMsgTimeoutOnCloseValidateBasic() {
}{
{"success", types.NewMsgTimeoutOnClose(packet, 1, suite.proof, suite.proof, height, addr), true},
{"seq 0", types.NewMsgTimeoutOnClose(packet, 0, suite.proof, suite.proof, height, addr), false},
{"empty proof", types.NewMsgTimeoutOnClose(packet, 1, emptyProof, suite.proof, height, addr), false},
{"empty proof close", types.NewMsgTimeoutOnClose(packet, 1, suite.proof, emptyProof, height, addr), false},
{"signer address is empty", types.NewMsgTimeoutOnClose(packet, 1, suite.proof, suite.proof, height, emptyAddr), false},
{"invalid packet", types.NewMsgTimeoutOnClose(invalidPacket, 1, suite.proof, suite.proof, height, addr), false},
}
Expand Down Expand Up @@ -410,7 +400,6 @@ func (suite *TypesTestSuite) TestMsgAcknowledgementValidateBasic() {
{"success", types.NewMsgAcknowledgement(packet, packet.GetData(), suite.proof, height, addr), true},
{"empty ack", types.NewMsgAcknowledgement(packet, nil, suite.proof, height, addr), false},
{"missing signer address", types.NewMsgAcknowledgement(packet, packet.GetData(), suite.proof, height, emptyAddr), false},
{"cannot submit an empty proof", types.NewMsgAcknowledgement(packet, packet.GetData(), emptyProof, height, addr), false},
{"invalid packet", types.NewMsgAcknowledgement(invalidPacket, packet.GetData(), suite.proof, height, addr), false},
}

Expand Down
16 changes: 16 additions & 0 deletions modules/light-clients/07-tendermint/client_state.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,14 @@ func (cs ClientState) VerifyMembership(
path exported.Path,
value []byte,
) error {
if height.IsZero() {
return sdkerrors.Wrap(sdkerrors.ErrInvalidHeight, "proof height must be non-zero")
}

if len(proof) == 0 {
return sdkerrors.Wrap(commitmenttypes.ErrInvalidProof, "cannot submit an empty proof")
}

if cs.GetLatestHeight().LT(height) {
return sdkerrors.Wrapf(
sdkerrors.ErrInvalidHeight,
Expand Down Expand Up @@ -264,6 +272,14 @@ func (cs ClientState) VerifyNonMembership(
proof []byte,
path exported.Path,
) error {
if height.IsZero() {
return sdkerrors.Wrap(sdkerrors.ErrInvalidHeight, "proof height must be non-zero")
}

if len(proof) == 0 {
return sdkerrors.Wrap(commitmenttypes.ErrInvalidProof, "cannot submit an empty proof")
}

if cs.GetLatestHeight().LT(height) {
return sdkerrors.Wrapf(
sdkerrors.ErrInvalidHeight,
Expand Down
12 changes: 12 additions & 0 deletions modules/light-clients/07-tendermint/client_state_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,12 @@ func (suite *TendermintTestSuite) TestVerifyMembership() {
value = []byte("invalid value")
}, false,
},
{
"proof is empty", func() {
// change the inserted proof
proof = []byte{}
}, false,
},
}

for _, tc := range testCases {
Expand Down Expand Up @@ -631,6 +637,12 @@ func (suite *TendermintTestSuite) TestVerifyNonMembership() {
proof, proofHeight = suite.chainB.QueryProof(key)
}, false,
},
{
"proof is empty", func() {
// change the inserted proof
proof = []byte{}
}, false,
},
}

for _, tc := range testCases {
Expand Down

0 comments on commit 3715153

Please sign in to comment.