From 378ced35742f36873cf8cc46888b3892aee1042f Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Tue, 15 Oct 2024 11:45:19 +0200 Subject: [PATCH 1/9] refactor: check timeouts directly --- modules/core/04-channel/v2/keeper/packet.go | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/modules/core/04-channel/v2/keeper/packet.go b/modules/core/04-channel/v2/keeper/packet.go index a3ac7e3401e..ca956a02a6e 100644 --- a/modules/core/04-channel/v2/keeper/packet.go +++ b/modules/core/04-channel/v2/keeper/packet.go @@ -67,10 +67,9 @@ func (k *Keeper) sendPacket( if err != nil { return 0, "", err } - // check if packet is timed out on the receiving chain - timeout := channeltypes.NewTimeoutWithTimestamp(timeoutTimestamp) - if timeout.TimestampElapsed(latestTimestamp) { - return 0, "", errorsmod.Wrap(timeout.ErrTimeoutElapsed(latestHeight, latestTimestamp), "invalid packet timeout") + + if timeoutTimestamp < latestTimestamp { + return 0, "", errorsmod.Wrapf(channeltypes.ErrTimeoutElapsed, "latest timestamp: %d, timeout timestamp: %d", latestTimestamp, timeoutTimestamp) } commitment := channeltypesv2.CommitPacket(packet) @@ -117,10 +116,9 @@ func (k *Keeper) recvPacket( // check if packet timed out by comparing it with the latest height of the chain sdkCtx := sdk.UnwrapSDKContext(ctx) - selfHeight, selfTimestamp := clienttypes.GetSelfHeight(ctx), uint64(sdkCtx.BlockTime().UnixNano()) - timeout := channeltypes.NewTimeoutWithTimestamp(packet.GetTimeoutTimestamp()) - if timeout.Elapsed(selfHeight, selfTimestamp) { - return errorsmod.Wrap(timeout.ErrTimeoutElapsed(selfHeight, selfTimestamp), "packet timeout elapsed") + currentTimestamp := uint64(sdkCtx.BlockTime().Unix()) + if packet.GetTimeoutTimestamp() >= currentTimestamp { + return errorsmod.Wrapf(channeltypes.ErrTimeoutElapsed, "current timestamp: %d, timeout timestamp: %d", currentTimestamp, packet.GetTimeoutTimestamp()) } // REPLAY PROTECTION: Packet receipts will indicate that a packet has already been received @@ -251,9 +249,8 @@ func (k *Keeper) timeoutPacket( return err } - timeout := channeltypes.NewTimeoutWithTimestamp(packet.GetTimeoutTimestamp()) - if !timeout.Elapsed(clienttypes.ZeroHeight(), proofTimestamp) { - return errorsmod.Wrap(timeout.ErrTimeoutNotReached(proofHeight.(clienttypes.Height), proofTimestamp), "packet timeout not reached") + if packet.GetTimeoutTimestamp() < proofTimestamp { + return errorsmod.Wrapf(channeltypes.ErrTimeoutNotReached, "proof timestamp: %d, timeout timestamp: %d", proofTimestamp, packet.GetTimeoutTimestamp()) } // check that the commitment has not been cleared and that it matches the packet sent by relayer From 990fa7e533dc714a06204e74630c6b5d0f1318e9 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Thu, 17 Oct 2024 15:22:17 +0200 Subject: [PATCH 2/9] chore: add seconds to nanos conversion --- modules/core/04-channel/v2/keeper/packet.go | 29 +++++++++------------ modules/core/04-channel/v2/types/packet.go | 6 +++++ 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/modules/core/04-channel/v2/keeper/packet.go b/modules/core/04-channel/v2/keeper/packet.go index ca956a02a6e..7885ff7107a 100644 --- a/modules/core/04-channel/v2/keeper/packet.go +++ b/modules/core/04-channel/v2/keeper/packet.go @@ -68,8 +68,9 @@ func (k *Keeper) sendPacket( return 0, "", err } - if timeoutTimestamp < latestTimestamp { - return 0, "", errorsmod.Wrapf(channeltypes.ErrTimeoutElapsed, "latest timestamp: %d, timeout timestamp: %d", latestTimestamp, timeoutTimestamp) + timeout := channeltypesv2.TimeoutTimestampToNanos(timeoutTimestamp) + if timeout < latestTimestamp { + return 0, "", errorsmod.Wrapf(channeltypes.ErrTimeoutElapsed, "latest timestamp: %d, timeout timestamp: %d", latestTimestamp, packet.TimeoutTimestamp) } commitment := channeltypesv2.CommitPacket(packet) @@ -98,9 +99,6 @@ func (k *Keeper) recvPacket( proof []byte, proofHeight exported.Height, ) error { - // Lookup channel associated with destination channel ID and ensure - // that the packet was indeed sent by our counterparty by verifying - // packet sender is our channel's counterparty channel id. channel, ok := k.GetChannel(ctx, packet.DestinationChannel) if !ok { // TODO: figure out how aliasing will work when more than one packet data is sent. @@ -109,15 +107,18 @@ func (k *Keeper) recvPacket( return errorsmod.Wrap(types.ErrChannelNotFound, packet.DestinationChannel) } } + if channel.CounterpartyChannelId != packet.SourceChannel { return channeltypes.ErrInvalidChannelIdentifier } + clientID := channel.ClientId // check if packet timed out by comparing it with the latest height of the chain sdkCtx := sdk.UnwrapSDKContext(ctx) currentTimestamp := uint64(sdkCtx.BlockTime().Unix()) - if packet.GetTimeoutTimestamp() >= currentTimestamp { + timeout := channeltypesv2.TimeoutTimestampToNanos(packet.TimeoutTimestamp) + if timeout >= currentTimestamp { return errorsmod.Wrapf(channeltypes.ErrTimeoutElapsed, "current timestamp: %d, timeout timestamp: %d", currentTimestamp, packet.GetTimeoutTimestamp()) } @@ -160,8 +161,6 @@ func (k *Keeper) recvPacket( } func (k *Keeper) acknowledgePacket(ctx context.Context, packet channeltypesv2.Packet, acknowledgement channeltypesv2.Acknowledgement, proof []byte, proofHeight exported.Height) error { - // Lookup counterparty associated with our channel and ensure - // that the packet was indeed sent by our counterparty. channel, ok := k.GetChannel(ctx, packet.SourceChannel) if !ok { return errorsmod.Wrap(types.ErrChannelNotFound, packet.SourceChannel) @@ -170,6 +169,7 @@ func (k *Keeper) acknowledgePacket(ctx context.Context, packet channeltypesv2.Pa if channel.CounterpartyChannelId != packet.DestinationChannel { return channeltypes.ErrInvalidChannelIdentifier } + clientID := channel.ClientId commitment := k.GetPacketCommitment(ctx, packet.SourceChannel, packet.Sequence) @@ -228,19 +228,15 @@ func (k *Keeper) timeoutPacket( proof []byte, proofHeight exported.Height, ) error { - // Lookup counterparty associated with our channel and ensure - // that the packet was indeed sent by our counterparty. channel, ok := k.GetChannel(ctx, packet.SourceChannel) if !ok { - // TODO: figure out how aliasing will work when more than one packet data is sent. - channel, ok = k.convertV1Channel(ctx, packet.Data[0].SourcePort, packet.SourceChannel) - if !ok { - return errorsmod.Wrap(types.ErrChannelNotFound, packet.DestinationChannel) - } + return errorsmod.Wrap(types.ErrChannelNotFound, packet.DestinationChannel) } + if channel.CounterpartyChannelId != packet.DestinationChannel { return channeltypes.ErrInvalidChannelIdentifier } + clientID := channel.ClientId // check that timeout height or timeout timestamp has passed on the other end @@ -249,7 +245,8 @@ func (k *Keeper) timeoutPacket( return err } - if packet.GetTimeoutTimestamp() < proofTimestamp { + timeout := channeltypesv2.TimeoutTimestampToNanos(packet.TimeoutTimestamp) + if timeout < proofTimestamp { return errorsmod.Wrapf(channeltypes.ErrTimeoutNotReached, "proof timestamp: %d, timeout timestamp: %d", proofTimestamp, packet.GetTimeoutTimestamp()) } diff --git a/modules/core/04-channel/v2/types/packet.go b/modules/core/04-channel/v2/types/packet.go index 81dd8905e14..8d874072e76 100644 --- a/modules/core/04-channel/v2/types/packet.go +++ b/modules/core/04-channel/v2/types/packet.go @@ -2,6 +2,7 @@ package types import ( "strings" + "time" errorsmod "cosmossdk.io/errors" @@ -84,3 +85,8 @@ func (p Payload) Validate() error { } return nil } + +// TimeoutTimestampToNanos takes seconds as a uint64 and returns Unix nanoseconds as uint64. +func TimeoutTimestampToNanos(seconds uint64) uint64 { + return uint64(time.Unix(int64(seconds), 0).Unix()) +} From e050758f052536face3f6493df7626026ff8e123 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Thu, 17 Oct 2024 16:06:26 +0200 Subject: [PATCH 3/9] rm stupid if --- modules/core/04-channel/v2/types/packet.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/modules/core/04-channel/v2/types/packet.go b/modules/core/04-channel/v2/types/packet.go index 787c95a459d..f6764dcdc54 100644 --- a/modules/core/04-channel/v2/types/packet.go +++ b/modules/core/04-channel/v2/types/packet.go @@ -1,7 +1,6 @@ package types import ( - "math" "strings" "time" @@ -89,9 +88,5 @@ func (p Payload) Validate() error { // TimeoutTimestampToNanos takes seconds as a uint64 and returns Unix nanoseconds as uint64. func TimeoutTimestampToNanos(seconds uint64) uint64 { - if seconds >= math.MaxInt64 { - - } - return uint64(time.Unix(int64(seconds), 0).UnixNano()) } From a2e22fe7770e05f42578bd3c55d012cdd977dae4 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Mon, 21 Oct 2024 10:53:34 +0200 Subject: [PATCH 4/9] chore: regen protos --- modules/core/04-channel/v2/types/packet.pb.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/core/04-channel/v2/types/packet.pb.go b/modules/core/04-channel/v2/types/packet.pb.go index 7ec4ffaadb0..89288d6ef0c 100644 --- a/modules/core/04-channel/v2/types/packet.pb.go +++ b/modules/core/04-channel/v2/types/packet.pb.go @@ -69,7 +69,7 @@ type Packet struct { SourceChannel string `protobuf:"bytes,2,opt,name=source_channel,json=sourceChannel,proto3" json:"source_channel,omitempty"` // identifies the receiving chain. DestinationChannel string `protobuf:"bytes,3,opt,name=destination_channel,json=destinationChannel,proto3" json:"destination_channel,omitempty"` - // timeout timestamp after which the packet times out. + // timeout timestamp in seconds after which the packet times out. TimeoutTimestamp uint64 `protobuf:"varint,4,opt,name=timeout_timestamp,json=timeoutTimestamp,proto3" json:"timeout_timestamp,omitempty"` // a list of packet data, each one for a specific application. Data []PacketData `protobuf:"bytes,5,rep,name=data,proto3" json:"data"` From 3b158d40d4f29215a00ca62ca094b0555fe3225c Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Mon, 21 Oct 2024 16:37:25 +0200 Subject: [PATCH 5/9] Update modules/core/04-channel/v2/keeper/packet.go Co-authored-by: DimitrisJim --- modules/core/04-channel/v2/keeper/packet.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/core/04-channel/v2/keeper/packet.go b/modules/core/04-channel/v2/keeper/packet.go index 3d61830bba7..63b16511975 100644 --- a/modules/core/04-channel/v2/keeper/packet.go +++ b/modules/core/04-channel/v2/keeper/packet.go @@ -119,7 +119,7 @@ func (k *Keeper) recvPacket( currentTimestamp := uint64(sdkCtx.BlockTime().UnixNano()) timeout := types.TimeoutTimestampToNanos(packet.TimeoutTimestamp) if timeout <= currentTimestamp { - return errorsmod.Wrapf(channeltypes.ErrTimeoutElapsed, "current timestamp: %d, timeout timestamp: %d", currentTimestamp, packet.GetTimeoutTimestamp()) + return errorsmod.Wrapf(channeltypes.ErrTimeoutElapsed, "current timestamp: %d, timeout timestamp: %d", currentTimestamp, timeout) } // REPLAY PROTECTION: Packet receipts will indicate that a packet has already been received From 0d10de66d76e5cd49a53915f8429b1cd2be2795f Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Mon, 21 Oct 2024 16:38:20 +0200 Subject: [PATCH 6/9] Update modules/core/04-channel/v2/keeper/packet.go --- modules/core/04-channel/v2/keeper/packet.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/core/04-channel/v2/keeper/packet.go b/modules/core/04-channel/v2/keeper/packet.go index 63b16511975..788567255c2 100644 --- a/modules/core/04-channel/v2/keeper/packet.go +++ b/modules/core/04-channel/v2/keeper/packet.go @@ -70,7 +70,7 @@ func (k *Keeper) sendPacket( timeout := types.TimeoutTimestampToNanos(packet.TimeoutTimestamp) if timeout < latestTimestamp { - return 0, "", errorsmod.Wrapf(channeltypes.ErrTimeoutElapsed, "latest timestamp: %d, timeout timestamp: %d", latestTimestamp, packet.TimeoutTimestamp) + return 0, "", errorsmod.Wrapf(channeltypes.ErrTimeoutElapsed, "latest timestamp: %d, timeout timestamp: %d", latestTimestamp, timeout) } commitment := types.CommitPacket(packet) From 588810f901337e12fa63586e5fcddbf7ffe44e23 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Tue, 22 Oct 2024 14:01:31 +0200 Subject: [PATCH 7/9] chore: compare timeouts as was done in the olden days --- .../04-channel/v2/keeper/msg_server_test.go | 6 +++--- modules/core/04-channel/v2/keeper/packet.go | 18 +++++++++--------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/modules/core/04-channel/v2/keeper/msg_server_test.go b/modules/core/04-channel/v2/keeper/msg_server_test.go index 028489c8315..4f05d40aec8 100644 --- a/modules/core/04-channel/v2/keeper/msg_server_test.go +++ b/modules/core/04-channel/v2/keeper/msg_server_test.go @@ -84,7 +84,7 @@ func (suite *KeeperTestSuite) TestMsgSendPacket() { path = ibctesting.NewPath(suite.chainA, suite.chainB) path.SetupV2() - timeoutTimestamp = suite.chainA.GetTimeoutTimestamp() + timeoutTimestamp = suite.chainA.GetTimeoutTimestampSecs() payload = mockv2.NewMockPayload(mockv2.ModuleNameA, mockv2.ModuleNameB) expectedPacket = channeltypesv2.NewPacket(1, path.EndpointA.ChannelID, path.EndpointB.ChannelID, timeoutTimestamp, payload) @@ -201,7 +201,7 @@ func (suite *KeeperTestSuite) TestMsgRecvPacket() { path = ibctesting.NewPath(suite.chainA, suite.chainB) path.SetupV2() - timeoutTimestamp := suite.chainA.GetTimeoutTimestamp() + timeoutTimestamp := suite.chainA.GetTimeoutTimestampSecs() var err error packet, err = path.EndpointA.MsgSendPacket(timeoutTimestamp, mockv2.NewMockPayload(mockv2.ModuleNameA, mockv2.ModuleNameB)) @@ -393,7 +393,7 @@ func (suite *KeeperTestSuite) TestMsgAcknowledgement() { path = ibctesting.NewPath(suite.chainA, suite.chainB) path.SetupV2() - timeoutTimestamp := suite.chainA.GetTimeoutTimestamp() + timeoutTimestamp := suite.chainA.GetTimeoutTimestampSecs() var err error // Send packet from A to B diff --git a/modules/core/04-channel/v2/keeper/packet.go b/modules/core/04-channel/v2/keeper/packet.go index 788567255c2..c377de65932 100644 --- a/modules/core/04-channel/v2/keeper/packet.go +++ b/modules/core/04-channel/v2/keeper/packet.go @@ -68,9 +68,9 @@ func (k *Keeper) sendPacket( return 0, "", err } - timeout := types.TimeoutTimestampToNanos(packet.TimeoutTimestamp) - if timeout < latestTimestamp { - return 0, "", errorsmod.Wrapf(channeltypes.ErrTimeoutElapsed, "latest timestamp: %d, timeout timestamp: %d", latestTimestamp, timeout) + timeoutTimestamp = types.TimeoutTimestampToNanos(packet.TimeoutTimestamp) + if latestTimestamp >= timeoutTimestamp { + return 0, "", errorsmod.Wrapf(channeltypes.ErrTimeoutElapsed, "latest timestamp: %d, timeout timestamp: %d", latestTimestamp, timeoutTimestamp) } commitment := types.CommitPacket(packet) @@ -117,9 +117,9 @@ func (k *Keeper) recvPacket( // check if packet timed out by comparing it with the latest height of the chain sdkCtx := sdk.UnwrapSDKContext(ctx) currentTimestamp := uint64(sdkCtx.BlockTime().UnixNano()) - timeout := types.TimeoutTimestampToNanos(packet.TimeoutTimestamp) - if timeout <= currentTimestamp { - return errorsmod.Wrapf(channeltypes.ErrTimeoutElapsed, "current timestamp: %d, timeout timestamp: %d", currentTimestamp, timeout) + timeoutTimestamp := types.TimeoutTimestampToNanos(packet.TimeoutTimestamp) + if currentTimestamp >= timeoutTimestamp { + return errorsmod.Wrapf(channeltypes.ErrTimeoutElapsed, "current timestamp: %d, timeout timestamp: %d", currentTimestamp, timeoutTimestamp) } // REPLAY PROTECTION: Packet receipts will indicate that a packet has already been received @@ -295,9 +295,9 @@ func (k *Keeper) timeoutPacket( return err } - timeout := types.TimeoutTimestampToNanos(packet.TimeoutTimestamp) - if timeout > proofTimestamp { - return errorsmod.Wrapf(channeltypes.ErrTimeoutNotReached, "proof timestamp: %d, timeout timestamp: %d", proofTimestamp, timeout) + timeoutTimestamp := types.TimeoutTimestampToNanos(packet.TimeoutTimestamp) + if proofTimestamp < timeoutTimestamp { + return errorsmod.Wrapf(channeltypes.ErrTimeoutNotReached, "proof timestamp: %d, timeout timestamp: %d", proofTimestamp, timeoutTimestamp) } // check that the commitment has not been cleared and that it matches the packet sent by relayer From a38bb30c4f6a76baa626cd8d4a6e2d00f0503017 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Wed, 23 Oct 2024 14:58:19 +0200 Subject: [PATCH 8/9] chore: update comments and use secs in recvPacket Co-authored-by: Aditya <14364734+AdityaSripal@users.noreply.github.com> --- modules/core/04-channel/v2/keeper/packet.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/modules/core/04-channel/v2/keeper/packet.go b/modules/core/04-channel/v2/keeper/packet.go index c377de65932..7eb6123a31b 100644 --- a/modules/core/04-channel/v2/keeper/packet.go +++ b/modules/core/04-channel/v2/keeper/packet.go @@ -68,6 +68,8 @@ func (k *Keeper) sendPacket( return 0, "", err } + // client timestamps are in nanoseconds while packet timeouts are in seconds + // thus to compare them, we convert the packet timeout to nanoseconds timeoutTimestamp = types.TimeoutTimestampToNanos(packet.TimeoutTimestamp) if latestTimestamp >= timeoutTimestamp { return 0, "", errorsmod.Wrapf(channeltypes.ErrTimeoutElapsed, "latest timestamp: %d, timeout timestamp: %d", latestTimestamp, timeoutTimestamp) @@ -116,9 +118,8 @@ func (k *Keeper) recvPacket( // check if packet timed out by comparing it with the latest height of the chain sdkCtx := sdk.UnwrapSDKContext(ctx) - currentTimestamp := uint64(sdkCtx.BlockTime().UnixNano()) - timeoutTimestamp := types.TimeoutTimestampToNanos(packet.TimeoutTimestamp) - if currentTimestamp >= timeoutTimestamp { + currentTimestamp := uint64(sdkCtx.BlockTime().Unix()) + if currentTimestamp >= packet.TimeoutTimestamp { return errorsmod.Wrapf(channeltypes.ErrTimeoutElapsed, "current timestamp: %d, timeout timestamp: %d", currentTimestamp, timeoutTimestamp) } From 0692d82dcb9cc10bbb30426c40365749e09619fc Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Wed, 23 Oct 2024 15:17:26 +0200 Subject: [PATCH 9/9] chore: fix the compiler error breakage --- modules/core/04-channel/v2/keeper/packet.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/core/04-channel/v2/keeper/packet.go b/modules/core/04-channel/v2/keeper/packet.go index 7eb6123a31b..b4e0516307d 100644 --- a/modules/core/04-channel/v2/keeper/packet.go +++ b/modules/core/04-channel/v2/keeper/packet.go @@ -68,8 +68,8 @@ func (k *Keeper) sendPacket( return 0, "", err } - // client timestamps are in nanoseconds while packet timeouts are in seconds - // thus to compare them, we convert the packet timeout to nanoseconds + // client timestamps are in nanoseconds while packet timeouts are in seconds + // thus to compare them, we convert the packet timeout to nanoseconds timeoutTimestamp = types.TimeoutTimestampToNanos(packet.TimeoutTimestamp) if latestTimestamp >= timeoutTimestamp { return 0, "", errorsmod.Wrapf(channeltypes.ErrTimeoutElapsed, "latest timestamp: %d, timeout timestamp: %d", latestTimestamp, timeoutTimestamp) @@ -120,7 +120,7 @@ func (k *Keeper) recvPacket( sdkCtx := sdk.UnwrapSDKContext(ctx) currentTimestamp := uint64(sdkCtx.BlockTime().Unix()) if currentTimestamp >= packet.TimeoutTimestamp { - return errorsmod.Wrapf(channeltypes.ErrTimeoutElapsed, "current timestamp: %d, timeout timestamp: %d", currentTimestamp, timeoutTimestamp) + return errorsmod.Wrapf(channeltypes.ErrTimeoutElapsed, "current timestamp: %d, timeout timestamp: %d", currentTimestamp, packet.TimeoutTimestamp) } // REPLAY PROTECTION: Packet receipts will indicate that a packet has already been received