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

Add back timestamp precondition #755

Merged
merged 17 commits into from
Feb 28, 2023
Merged

Add back timestamp precondition #755

merged 17 commits into from
Feb 28, 2023

Conversation

mitschabaude
Copy link
Member

@mitschabaude mitschabaude commented Feb 27, 2023

Timestamp precondition RFC

network.timestamp is implemented as a wrapper for network.globalSlotSinceGenesis, which means:

  • when using network.timestamp.get(), the global slot is fetched under the hood and converted to a timestamp
  • when using timestamp.assertEquals() or timestamp.assertBetween(), a precondition for globalSlotSinceGenesis is added under the hood

Implementation details

  • conversion between timestamps and global slots requires the knowledge of the timestamp which corresponds to slot 0. This is called the "genesis timestamp" and is a hardcoded property of each deployed network. We expose this by adding a new method .getNetworkConstants() on the Mina interface, which returns constants such as the genesis timestamp. This way, we can also use a reasonable mock value for local blockchain: the timestamp at which LocalBlockchain is instantiated.
  • However, for remote networks, currently there is no way to obtain the genesis timestamp. This needs to be added to the GraphQL interface: add genesis-timestamp to graphQL endpoint genesisConstants MinaProtocol/mina#12728 . Since the GraphQL interface will only change upon upgrade of the current testnet, as an intermediate solution until that upgrade we hardcode the genesis timestamp of the current testnet as a constant.
  • Added some primitives for in-snark conversion of a UInt64 to a UInt32 with two possible semantics: either assert that the UInt64 fits in 32 bits already, or clamp the UInt64 to the uint32 range, so that larger values just become the max uint32. For our conversion of timestamp ranges to slot ranges, clamping is appropriate: this enables users to specify a "definitely large enough" timestamp value as the upper bound and have it converted to the max uint32 which is a definitely large enough slot.

Potential caveats

The fact that we implement one precondition on top of another means that the network doesn't know that timestamp was used and can't return an appropriate error. This is currently not an issue because the protocol returns Protocol_state_precondition_unsatisfied for all failing network preconditions, so it doesn't distinguish the exact case anyway.

@mitschabaude mitschabaude marked this pull request as ready for review February 28, 2023 09:57
@Trivo25
Copy link
Member

Trivo25 commented Feb 28, 2023

My only concern is dealing with long forks and missed slot, which will inevitable cause the timestamp to run out of sync with the "real world" time, which might give the user/developer a wrong understanding of how to use this.

Copy link
Member

@bkase bkase left a comment

Choose a reason for hiding this comment

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

This looks good to me! Maybe we can add docs to timestamp to let people know it’s virtualized on top of currentSlot

src/lib/precondition.ts Outdated Show resolved Hide resolved
@mitschabaude mitschabaude merged commit f38fafc into main Feb 28, 2023
@mitschabaude mitschabaude deleted the feature/timestamp branch February 28, 2023 13:42
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.

Add back timestamp precondition by wrapping global slot
3 participants