Skip to content

Commit

Permalink
Allow zero proof height packet recv ack timeout (#2781)
Browse files Browse the repository at this point in the history
  • Loading branch information
chatton authored Nov 22, 2022
1 parent 0ed02b7 commit 7cb3068
Show file tree
Hide file tree
Showing 6 changed files with 318 additions and 138 deletions.
12 changes: 0 additions & 12 deletions modules/core/04-channel/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,9 +323,6 @@ func (msg MsgRecvPacket) ValidateBasic() error {
if len(msg.ProofCommitment) == 0 {
return sdkerrors.Wrap(commitmenttypes.ErrInvalidProof, "cannot submit an empty proof")
}
if msg.ProofHeight.IsZero() {
return sdkerrors.Wrap(sdkerrors.ErrInvalidHeight, "proof height must be non-zero")
}
_, 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 @@ -372,9 +369,6 @@ func (msg MsgTimeout) ValidateBasic() error {
if len(msg.ProofUnreceived) == 0 {
return sdkerrors.Wrap(commitmenttypes.ErrInvalidProof, "cannot submit an empty unreceived proof")
}
if msg.ProofHeight.IsZero() {
return sdkerrors.Wrap(sdkerrors.ErrInvalidHeight, "proof height must be non-zero")
}
if msg.NextSequenceRecv == 0 {
return sdkerrors.Wrap(sdkerrors.ErrInvalidSequence, "next sequence receive cannot be 0")
}
Expand Down Expand Up @@ -423,9 +417,6 @@ func (msg MsgTimeoutOnClose) ValidateBasic() error {
if len(msg.ProofClose) == 0 {
return sdkerrors.Wrap(commitmenttypes.ErrInvalidProof, "cannot submit an empty proof of closed counterparty channel end")
}
if msg.ProofHeight.IsZero() {
return sdkerrors.Wrap(sdkerrors.ErrInvalidHeight, "proof height must be non-zero")
}
_, 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 @@ -467,9 +458,6 @@ func (msg MsgAcknowledgement) ValidateBasic() error {
if len(msg.ProofAcked) == 0 {
return sdkerrors.Wrap(commitmenttypes.ErrInvalidProof, "cannot submit an empty proof")
}
if msg.ProofHeight.IsZero() {
return sdkerrors.Wrap(sdkerrors.ErrInvalidHeight, "proof height must be non-zero")
}
if len(msg.Acknowledgement) == 0 {
return sdkerrors.Wrap(ErrInvalidAcknowledgement, "ack bytes cannot be empty")
}
Expand Down
4 changes: 0 additions & 4 deletions modules/core/04-channel/types/msgs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,6 @@ func (suite *TypesTestSuite) TestMsgRecvPacketValidateBasic() {
expPass bool
}{
{"success", types.NewMsgRecvPacket(packet, suite.proof, height, addr), true},
{"proof height is zero", types.NewMsgRecvPacket(packet, suite.proof, clienttypes.ZeroHeight(), addr), false},
{"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 @@ -351,7 +350,6 @@ func (suite *TypesTestSuite) TestMsgTimeoutValidateBasic() {
expPass bool
}{
{"success", types.NewMsgTimeout(packet, 1, suite.proof, height, addr), true},
{"proof height must be > 0", types.NewMsgTimeout(packet, 1, suite.proof, clienttypes.ZeroHeight(), addr), false},
{"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},
Expand Down Expand Up @@ -383,7 +381,6 @@ func (suite *TypesTestSuite) TestMsgTimeoutOnCloseValidateBasic() {
{"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},
{"proof height is zero", types.NewMsgTimeoutOnClose(packet, 1, suite.proof, suite.proof, clienttypes.ZeroHeight(), 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 All @@ -410,7 +407,6 @@ func (suite *TypesTestSuite) TestMsgAcknowledgementValidateBasic() {
expPass bool
}{
{"success", types.NewMsgAcknowledgement(packet, packet.GetData(), suite.proof, height, addr), true},
{"proof height must be > 0", types.NewMsgAcknowledgement(packet, packet.GetData(), suite.proof, clienttypes.ZeroHeight(), addr), false},
{"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},
Expand Down
3 changes: 0 additions & 3 deletions modules/light-clients/06-solomachine/client_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,6 @@ func (cs ClientState) GetTimestampAtHeight(
cdc codec.BinaryCodec,
height exported.Height,
) (uint64, error) {
if !cs.GetLatestHeight().EQ(height) {
return 0, sdkerrors.Wrapf(ErrInvalidSequence, "not latest height (%s)", height)
}
return cs.ConsensusState.Timestamp, nil
}

Expand Down
13 changes: 4 additions & 9 deletions modules/light-clients/06-solomachine/client_state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() {
)

commitmentBz := channeltypes.CommitPacket(suite.chainA.Codec, packet)
path = sm.GetPacketCommitmentPath(ibctesting.MockPort, ibctesting.FirstChannelID)
path = sm.GetPacketCommitmentPath(packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())
signBytes = solomachine.SignBytes{
Sequence: sm.Sequence,
Timestamp: sm.Time,
Expand All @@ -372,7 +372,7 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() {
{
"success: packet acknowledgement verification",
func() {
path = sm.GetPacketAcknowledgementPath(ibctesting.MockPort, ibctesting.FirstChannelID)
path = sm.GetPacketAcknowledgementPath(ibctesting.MockPort, ibctesting.FirstChannelID, 1)
signBytes = solomachine.SignBytes{
Sequence: sm.Sequence,
Timestamp: sm.Time,
Expand All @@ -399,7 +399,7 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() {
{
"success: packet receipt verification",
func() {
path = sm.GetPacketReceiptPath(ibctesting.MockPort, ibctesting.FirstChannelID)
path = sm.GetPacketReceiptPath(ibctesting.MockPort, ibctesting.FirstChannelID, 1)
signBytes = solomachine.SignBytes{
Sequence: sm.Sequence,
Timestamp: sm.Time,
Expand Down Expand Up @@ -608,7 +608,7 @@ func (suite *SoloMachineTestSuite) TestVerifyNonMembership() {
{
"success: packet receipt absence verification",
func() {
path = suite.solomachine.GetPacketReceiptPath(ibctesting.MockPort, ibctesting.FirstChannelID)
path = suite.solomachine.GetPacketReceiptPath(ibctesting.MockPort, ibctesting.FirstChannelID, 1)
signBytes = solomachine.SignBytes{
Sequence: sm.GetHeight().GetRevisionHeight(),
Timestamp: sm.Time,
Expand Down Expand Up @@ -794,11 +794,6 @@ func (suite *SoloMachineTestSuite) TestGetTimestampAtHeight() {
expValue: suite.solomachine.ClientState().ConsensusState.Timestamp,
expPass: true,
},
{
name: "get timestamp at height not exists",
clientState: suite.solomachine.ClientState(),
height: suite.solomachine.ClientState().GetLatestHeight().Increment(),
},
}

for i, tc := range testCases {
Expand Down
96 changes: 95 additions & 1 deletion modules/light-clients/06-solomachine/solomachine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package solomachine_test

import (
"testing"
"time"

codectypes "github.com/cosmos/cosmos-sdk/codec/types"
cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec"
Expand All @@ -11,10 +12,17 @@ import (
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"

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"
"github.com/cosmos/ibc-go/v6/modules/core/exported"
solomachine "github.com/cosmos/ibc-go/v6/modules/light-clients/06-solomachine"
ibctesting "github.com/cosmos/ibc-go/v6/testing"
"github.com/cosmos/ibc-go/v6/testing/mock"
)

var (
channelIDSolomachine = "channel-on-solomachine" // channelID generated on solo machine side
)

type SoloMachineTestSuite struct {
Expand Down Expand Up @@ -46,7 +54,7 @@ func TestSoloMachineTestSuite(t *testing.T) {
suite.Run(t, new(SoloMachineTestSuite))
}

func (suite *SoloMachineTestSuite) TestSolomachineSetup() {
func (suite *SoloMachineTestSuite) SetupSolomachine() string {
clientID := suite.solomachine.CreateClient(suite.chainA)

connectionID := suite.solomachine.ConnOpenInit(suite.chainA, clientID)
Expand All @@ -65,11 +73,97 @@ func (suite *SoloMachineTestSuite) TestSolomachineSetup() {

// open confirm is not necessary as the solo machine implementation is mocked

return channelID
}

func (suite *SoloMachineTestSuite) TestRecvPacket() {
channelID := suite.SetupSolomachine()

packet := channeltypes.NewPacket(
mock.MockPacketData,
1,
mock.PortID,
channelIDSolomachine,
mock.PortID,
channelID,
clienttypes.ZeroHeight(),
uint64(suite.chainA.GetContext().BlockTime().Add(time.Hour).UnixNano()),
)

// send packet is not necessary as the solo machine implementation is mocked

suite.solomachine.RecvPacket(suite.chainA, packet)

// close init is not necessary as the solomachine implementation is mocked

suite.solomachine.ChanCloseConfirm(suite.chainA, channelID)
}

func (suite *SoloMachineTestSuite) TestAcknowledgePacket() {
channelID := suite.SetupSolomachine()

packet := channeltypes.NewPacket(
mock.MockPacketData,
1,
mock.PortID,
channelID,
mock.PortID,
channelIDSolomachine,
clienttypes.ZeroHeight(),
uint64(suite.chainA.GetContext().BlockTime().Add(time.Hour).UnixNano()),
)

suite.solomachine.SendPacket(suite.chainA, packet)

// recv packet is not necessary as the solo machine implementation is mocked

suite.solomachine.AcknowledgePacket(suite.chainA, packet)

// close init is not necessary as the solomachine implementation is mocked

suite.solomachine.ChanCloseConfirm(suite.chainA, channelID)
}

func (suite *SoloMachineTestSuite) TestTimeout() {
channelID := suite.SetupSolomachine()

packet := channeltypes.NewPacket(
mock.MockPacketData,
1,
mock.PortID,
channelID,
mock.PortID,
channelIDSolomachine,
clienttypes.ZeroHeight(),
1,
)

suite.solomachine.SendPacket(suite.chainA, packet)

suite.solomachine.TimeoutPacket(suite.chainA, packet)

suite.solomachine.ChanCloseConfirm(suite.chainA, channelID)
}

func (suite *SoloMachineTestSuite) TestTimeoutOnClose() {
channelID := suite.SetupSolomachine()

packet := channeltypes.NewPacket(
mock.MockPacketData,
1,
mock.PortID,
channelID,
mock.PortID,
channelIDSolomachine,
clienttypes.ZeroHeight(),
1,
)

suite.solomachine.SendPacket(suite.chainA, packet)

suite.solomachine.TimeoutPacketOnClose(suite.chainA, packet, channelID)
}

func (suite *SoloMachineTestSuite) GetSequenceFromStore() uint64 {
bz := suite.store.Get(host.ClientStateKey())
suite.Require().NotNil(bz)
Expand Down
Loading

0 comments on commit 7cb3068

Please sign in to comment.