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

Make time conversion infrastructure available through the local state query protocol #2365

Closed
edsko opened this issue Jul 7, 2020 · 1 comment · Fixed by #2370
Closed
Assignees
Labels
consensus issues related to ouroboros-consensus daedalus When the wallet switches from cardano-sl to the new node

Comments

@edsko
Copy link
Contributor

edsko commented Jul 7, 2020

Tooling such as the wallet need access to time conversions, and it makes sense to make this available through the LSQ protocol, because the HFC goes through great lengths to track the necessary information. Since tooling might need to do many conversions, it might make sense to make a LSQ query available that returns an interpreter for Qry (from Ouroboros.Consensus.HardFork.History.Qry). Tooling should be aware however that such an interpreter will have a limited range, and will need to be updated regularly.

@edsko edsko added consensus issues related to ouroboros-consensus shelley mainnet labels Jul 7, 2020
@edsko
Copy link
Contributor Author

edsko commented Jul 7, 2020

Incidentally, we have been wondering whether the simplification that the consensus layer makes that time conversions should not be subject to rollback is perhaps not strictly required, thereby giving is the possibility of removing the requirement for the double stability window. If we are now making time conversions available to tooling (such as the wallet), that simplifying assumption becomes more important, I think. Perhaps we can check our uses of conversions and verify that if they are subject to rollback it’s fine, but now that we are making this available to tooling, I much prefer to be able to say “yes, we know the answer for sure, here it is” or “we don’t know that yet” rather than “here’s a guess, but be careful, if we rollback, it might change”.

@mrBliss mrBliss added the daedalus When the wallet switches from cardano-sl to the new node label Jul 7, 2020
@mrBliss mrBliss added this to the S17 2020-07-16 milestone Jul 7, 2020
mrBliss added a commit that referenced this issue Jul 8, 2020
Fixes #2365.

* Introduce `QueryHardFork` for queries specific to the hard fork combinator,
  unrelated to any specific era. Provide a query to ask for an `Interpreter`,
  which can convert a slot to an epoch or time, etc.

* Generalise `QueryAnytime` so that the first era can also be queried. We
  still require there to be more than one era. Add the `QueryAnytimeByron`
  pattern synonym.

* Rename the `EraStart` constructor of `QueryAnytime` to `GetEraStart` for
  consistency.

* Change the encoding format of hard fork queries. Since they are not part of
  the final Shelley release yet (right?!), this is fine.
iohk-bors bot added a commit that referenced this issue Jul 8, 2020
2370: Introduce QueryHardFork r=mrBliss a=mrBliss

Fixes #2365.

* Introduce `QueryHardFork` for queries specific to the hard fork combinator,
  unrelated to any specific era. Provide a query to ask for an `Interpreter`,
  which can convert a slot to an epoch or time, etc.

* Generalise `QueryAnytime` so that the first era can also be queried. We
  still require there to be more than one era. Add the `QueryAnytimeByron`
  pattern synonym.

* Rename the `EraStart` constructor of `QueryAnytime` to `GetEraStart` for
  consistency.

* Change the encoding format of hard fork queries. Since they are not part of
  the final Shelley release yet (right?!), this is fine.


Co-authored-by: Thomas Winant <[email protected]>
@iohk-bors iohk-bors bot closed this as completed in 43a6ab9 Jul 8, 2020
@iohk-bors iohk-bors bot closed this as completed in #2370 Jul 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consensus issues related to ouroboros-consensus daedalus When the wallet switches from cardano-sl to the new node
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants