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

feat: add timestamps parameter #126

Merged
merged 47 commits into from
Nov 28, 2023

Conversation

fahimahmedx
Copy link
Contributor

@fahimahmedx fahimahmedx commented Nov 15, 2023

Motivation

Be able to filter for data by using a timestamp parameter. Initially this will be a unix timestamp.
Example usage:
cryo logs --timestamps 1700080000:1700090000

Solution

  • Add timestamp to args.rs
  • Create timestamp_to_block_number() function that gives the nearest block number for a given timestamp.
  • Add parse_timestamp() and related functions that parse an inputted timestamp.
  • Change timestamp_to_block_number() to be strictly <= instead of closest block or <= if two blocks are equidistant.
  • Refactor get_latest_block_number() into blocks.rs
  • Add tests
  • Add documentation

@fahimahmedx fahimahmedx changed the title feat: add Timestamp Parameter feat: add timestamp parameter Nov 15, 2023
@sslivkoff
Copy link
Member

looking great. thanks for taking this on

@sslivkoff
Copy link
Member

some important edge cases to check:

  • a timestamp less than genesis timestamp
  • a timestamp equal to genesis timestamp
  • a timestamp greater than latest block timestamp
  • a timestamp equal to latest block timestamp

@sslivkoff
Copy link
Member

the formatting error gets fixed with cargo +nightly fmt --all

@fahimahmedx fahimahmedx changed the title feat: add timestamp parameter feat: add timestamps parameter Nov 27, 2023
@fahimahmedx fahimahmedx marked this pull request as ready for review November 27, 2023 22:55
Copy link
Member

@sslivkoff sslivkoff left a comment

Choose a reason for hiding this comment

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

nice job. this is looking really good and it will be a very useful feature. I like that you implemented the full parsing grammar in the block style syntax

I think it's ready to merge after the 2 small points I highlighted

crates/cli/src/parse/timestamps.rs Outdated Show resolved Hide resolved
crates/cli/src/parse/timestamps.rs Show resolved Hide resolved
@sslivkoff sslivkoff merged commit c5e4490 into paradigmxyz:main Nov 28, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants