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

Compare Timeout timestamps in seconds instead of nanoseconds #7857

Merged
merged 4 commits into from
Jan 20, 2025
Merged
Show file tree
Hide file tree
Changes from 2 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
20 changes: 13 additions & 7 deletions modules/core/04-channel/v2/keeper/packet.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"bytes"
"context"
"strconv"
"time"

errorsmod "cosmossdk.io/errors"

Expand Down Expand Up @@ -53,15 +54,17 @@
return 0, "", errorsmod.Wrapf(clienttypes.ErrInvalidHeight, "cannot send packet using client (%s) with zero height", sourceClient)
}

// client timestamps are in nanoseconds while packet timeouts are in seconds
// thus to compare them, we convert the client timestamp to seconds in uint64
// to be consistent with IBC V2 specified timeout behaviour
latestTimestamp, err := k.ClientKeeper.GetClientTimestampAtHeight(ctx, sourceClient, latestHeight)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: would be slightly easier to read the code below if this was called latestTimestampNano (and would avoid accidental usage later)

if err != nil {
return 0, "", err
}
latestTimestampSecs := uint64(time.Unix(0, int64(latestTimestamp)).Unix())

// 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)

Check failure on line 66 in modules/core/04-channel/v2/keeper/packet.go

View workflow job for this annotation

GitHub Actions / lint

undefined: types.TimeoutTimestampToNanos) (typecheck)

Check failure on line 66 in modules/core/04-channel/v2/keeper/packet.go

View workflow job for this annotation

GitHub Actions / lint

undefined: types.TimeoutTimestampToNanos) (typecheck)

Check failure on line 66 in modules/core/04-channel/v2/keeper/packet.go

View workflow job for this annotation

GitHub Actions / lint

undefined: types.TimeoutTimestampToNanos) (typecheck)

Check failure on line 66 in modules/core/04-channel/v2/keeper/packet.go

View workflow job for this annotation

GitHub Actions / lint

undefined: types.TimeoutTimestampToNanos) (typecheck)

Check failure on line 66 in modules/core/04-channel/v2/keeper/packet.go

View workflow job for this annotation

GitHub Actions / lint

undefined: types.TimeoutTimestampToNanos) (typecheck)

Check failure on line 66 in modules/core/04-channel/v2/keeper/packet.go

View workflow job for this annotation

GitHub Actions / lint

undefined: types.TimeoutTimestampToNanos) (typecheck)

Check failure on line 66 in modules/core/04-channel/v2/keeper/packet.go

View workflow job for this annotation

GitHub Actions / tests (03)

undefined: types.TimeoutTimestampToNanos

Check failure on line 66 in modules/core/04-channel/v2/keeper/packet.go

View workflow job for this annotation

GitHub Actions / tests (00)

undefined: types.TimeoutTimestampToNanos

Check failure on line 66 in modules/core/04-channel/v2/keeper/packet.go

View workflow job for this annotation

GitHub Actions / tests (01)

undefined: types.TimeoutTimestampToNanos

Check failure on line 66 in modules/core/04-channel/v2/keeper/packet.go

View workflow job for this annotation

GitHub Actions / tests (01)

undefined: types.TimeoutTimestampToNanos

Check failure on line 66 in modules/core/04-channel/v2/keeper/packet.go

View workflow job for this annotation

GitHub Actions / tests (02)

undefined: types.TimeoutTimestampToNanos

Check failure on line 66 in modules/core/04-channel/v2/keeper/packet.go

View workflow job for this annotation

GitHub Actions / tests (02)

undefined: types.TimeoutTimestampToNanos

Check failure on line 66 in modules/core/04-channel/v2/keeper/packet.go

View workflow job for this annotation

GitHub Actions / tests (02)

undefined: types.TimeoutTimestampToNanos

Check failure on line 66 in modules/core/04-channel/v2/keeper/packet.go

View workflow job for this annotation

GitHub Actions / tests (02)

undefined: types.TimeoutTimestampToNanos

Check failure on line 66 in modules/core/04-channel/v2/keeper/packet.go

View workflow job for this annotation

GitHub Actions / tests (02)

undefined: types.TimeoutTimestampToNanos

Check failure on line 66 in modules/core/04-channel/v2/keeper/packet.go

View workflow job for this annotation

GitHub Actions / tests (02)

undefined: types.TimeoutTimestampToNanos

Check failure on line 66 in modules/core/04-channel/v2/keeper/packet.go

View workflow job for this annotation

GitHub Actions / tests (02)

undefined: types.TimeoutTimestampToNanos

Check failure on line 66 in modules/core/04-channel/v2/keeper/packet.go

View workflow job for this annotation

GitHub Actions / tests (02)

undefined: types.TimeoutTimestampToNanos

Check failure on line 66 in modules/core/04-channel/v2/keeper/packet.go

View workflow job for this annotation

GitHub Actions / tests (02)

undefined: types.TimeoutTimestampToNanos
if latestTimestamp >= timeoutTimestamp {
if latestTimestampSecs >= packet.TimeoutTimestamp {
return 0, "", errorsmod.Wrapf(types.ErrTimeoutElapsed, "latest timestamp: %d, timeout timestamp: %d", latestTimestamp, timeoutTimestamp)
}

Expand Down Expand Up @@ -264,15 +267,18 @@
return errorsmod.Wrapf(clienttypes.ErrInvalidCounterparty, "counterparty id (%s) does not match packet destination id (%s)", counterparty.ClientId, packet.DestinationClient)
}

// check that timeout height or timeout timestamp has passed on the other end
// check that timeout timestamp has passed on the other end
// client timestamps are in nanoseconds while packet timeouts are in seconds
// so we convert client timestamp to seconds in uint64 to be consistent
// with IBC V2 timeout behaviour
proofTimestamp, err := k.ClientKeeper.GetClientTimestampAtHeight(ctx, packet.SourceClient, proofHeight)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

if err != nil {
return err
}
proofTimestampSecs := uint64(time.Unix(0, int64(proofTimestamp)).Unix())

timeoutTimestamp := types.TimeoutTimestampToNanos(packet.TimeoutTimestamp)
if proofTimestamp < timeoutTimestamp {
return errorsmod.Wrapf(types.ErrTimeoutNotReached, "proof timestamp: %d, timeout timestamp: %d", proofTimestamp, timeoutTimestamp)
if proofTimestampSecs < packet.TimeoutTimestamp {
return errorsmod.Wrapf(types.ErrTimeoutNotReached, "proof timestamp: %d, timeout timestamp: %d", proofTimestamp, packet.TimeoutTimestamp)
}

// check that the commitment has not been cleared and that it matches the packet sent by relayer
Expand Down
6 changes: 0 additions & 6 deletions modules/core/04-channel/v2/types/packet.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package types

import (
"strings"
"time"

errorsmod "cosmossdk.io/errors"

Expand Down Expand Up @@ -79,8 +78,3 @@ func (p Payload) ValidateBasic() 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).UnixNano())
}
Loading