From 371515391f34b1714a23560a3a5c8a9de0270164 Mon Sep 17 00:00:00 2001 From: Charly Date: Thu, 2 Feb 2023 16:32:55 +0100 Subject: [PATCH] Remove ProofHeight and Proof Validation in msg.ValidateBasic (#3061) --- modules/core/04-channel/types/msgs.go | 28 ------------------- modules/core/04-channel/types/msgs_test.go | 11 -------- .../07-tendermint/client_state.go | 16 +++++++++++ .../07-tendermint/client_state_test.go | 12 ++++++++ 4 files changed, 28 insertions(+), 39 deletions(-) diff --git a/modules/core/04-channel/types/msgs.go b/modules/core/04-channel/types/msgs.go index b3d9aaf0644..a617fb2b86f 100644 --- a/modules/core/04-channel/types/msgs.go +++ b/modules/core/04-channel/types/msgs.go @@ -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" ) @@ -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", @@ -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) @@ -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) @@ -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) @@ -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) @@ -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") } @@ -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) @@ -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") } diff --git a/modules/core/04-channel/types/msgs_test.go b/modules/core/04-channel/types/msgs_test.go index 1aa2690209b..1085267a8a1 100644 --- a/modules/core/04-channel/types/msgs_test.go +++ b/modules/core/04-channel/types/msgs_test.go @@ -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 @@ -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}, } @@ -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}, } @@ -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 { @@ -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 { @@ -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}, } @@ -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}, } @@ -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}, } @@ -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}, } diff --git a/modules/light-clients/07-tendermint/client_state.go b/modules/light-clients/07-tendermint/client_state.go index 5b083341415..9a74834a299 100644 --- a/modules/light-clients/07-tendermint/client_state.go +++ b/modules/light-clients/07-tendermint/client_state.go @@ -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, @@ -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, diff --git a/modules/light-clients/07-tendermint/client_state_test.go b/modules/light-clients/07-tendermint/client_state_test.go index 2a70d4199a6..9df8c6047ba 100644 --- a/modules/light-clients/07-tendermint/client_state_test.go +++ b/modules/light-clients/07-tendermint/client_state_test.go @@ -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 { @@ -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 {