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

Consider converting proof timestamps to secs and floor (ignore nanos) #7512

Closed
damiannolan opened this issue Oct 23, 2024 · 4 comments · Fixed by #7857
Closed

Consider converting proof timestamps to secs and floor (ignore nanos) #7512

damiannolan opened this issue Oct 23, 2024 · 4 comments · Fixed by #7857
Assignees
Labels
feat: ibc-eureka needs discussion Issues that need discussion before they can be worked on

Comments

@damiannolan
Copy link
Contributor

damiannolan commented Oct 23, 2024

Since other implementations may just use seconds, I'm wondering if we should instead of using the block time and client timestamp in nanos actually just convert them to seconds and floor it (ie ignore subseconds) to be consistent

In practice, this is a non-issue but worth noting and deciding on before release

Originally posted by @AdityaSripal in #7464 (review)

@damiannolan
Copy link
Contributor Author

Note we can do this pretty easily by creating a time.Time with time.Unix(), passing the full prootTimestamp as num nanos, and then calling .Unix()

secsTimestamp := time.Unix(0, proofTimestamp).Unix()

@gjermundgaraba
Copy link
Contributor

gjermundgaraba commented Nov 18, 2024

Just to be clear, is this issue about potentially changing the light clients to use seconds instead of nanoseconds?
@AdityaSripal

Also, is this a Eureka issue?

@gjermundgaraba gjermundgaraba added needs discussion Issues that need discussion before they can be worked on feat: ibc-eureka labels Nov 18, 2024
@AdityaSripal
Copy link
Member

We probably will not change the ibc-go light clients directly since we want to still support IBC v1 and minimize changes.

Rather this is about how to convert the nanoseconds from the client to seconds in the v2 handler correctly

@srdtrk
Copy link
Member

srdtrk commented Jan 21, 2025

closed by #7857

@srdtrk srdtrk closed this as completed Jan 21, 2025
@github-project-automation github-project-automation bot moved this from Backlog to Done in IBC-GO Eureka Jan 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat: ibc-eureka needs discussion Issues that need discussion before they can be worked on
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants