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.

Sign up for GitHub

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

Jump to bottom

Provide more information on failure to determine current epoch. #1939

Closed
1 task done
jonathanknowles opened this issue Jul 23, 2020 · 2 comments
Closed
1 task done

Provide more information on failure to determine current epoch. #1939

jonathanknowles opened this issue Jul 23, 2020 · 2 comments
Assignees
@jonathanknowles
Milestone
(ADP-313) Track S...

Comments

@jonathanknowles
Copy link
Member

jonathanknowles commented Jul 23, 2020 •
edited
Loading

Context

As part of the work to track stake pool retirements, we must reject attempts to join stake pools that have already retired.

In order to do this, we must compare the pool's retirement epoch with the current epoch.

However, determining the current epoch is an operation that might fail:

https://github.com/input-output-hk/cardano-wallet/blob/05d377418b02e59e1299a0eaee135802c52735d2/lib/core/src/Cardano/Wallet/Primitive/Slotting.hs#L108-L109

If we encounter this condition, we currently report the following error:

https://github.com/input-output-hk/cardano-wallet/blob/05d377418b02e59e1299a0eaee135802c52735d2/lib/core/src/Cardano/Wallet/Api/Server.hs#L1902-L1906

Decision

Determine the conditions under which this operation may fail, and if necessary, update the error message to be more informative.

Acceptance Criteria

  • The error message associated with ErrUnableToDetermineCurrentEpoch should describe a course of action for the user to take.

Development

See #1992

QA

See #1992
@jonathanknowles jonathanknowles self-assigned this Jul 23, 2020
@jonathanknowles jonathanknowles mentioned this issue Jul 23, 2020
Merged
1 task
@jonathanknowles
Copy link
Member Author

@Anviking Do you have any opinion on how we might make this particular error more informative?

@jonathanknowles jonathanknowles added this to Backlog in Adrestia via automation Jul 25, 2020
@Anviking
Copy link
Collaborator

It's possible with #1943, with the change from TimeInterpreter IO to TimeInterpreter (ExceptT IO PastHorizonException) (Sorry, I think I indicated some time that the Nothing of currentEpoch could "include" PastHorizonException, but that is not the case.)

There should be reasons for failure:

  • Current time is behind genesis. Action: 🤷‍♂️
  • Node is in the Byron era of Byron;Shelley, and the current time is beyond our forecast zone. Action: Wait for the node to sync more

A concrete example of PastHorizonException behaviour here, in case it is useful for context.

@jonathanknowles jonathanknowles mentioned this issue Aug 3, 2020
Merged
iohk-bors bot added a commit that referenced this issue Aug 3, 2020
@iohk-bors @jonathanknowles
Merge #1992
d51b74d 
1992: Add user advice to `UnableToDetermineCurrentEpoch` error. r=jonathanknowles a=jonathanknowles

# Issue Number

#1939 

# Overview

This PR revises the error we show if we cannot determine the current epoch, inviting the user to wait and try again.

# Comments

Ideally, we'd be able to give the user some indication of _how long_ they should wait, but assuming we cannot reliably know this information, letting them know that waiting may resolve the issue is arguably better than giving no advice at all.


Co-authored-by: Jonathan Knowles <jonathan.knowles@iohk.io>
@jonathanknowles jonathanknowles moved this from Backlog to QA in Adrestia Aug 3, 2020
@jonathanknowles jonathanknowles moved this from QA to Closed in Adrestia Aug 5, 2020
@CharlesMorgan-IOHK CharlesMorgan-IOHK removed this from Closed in Adrestia Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jonathanknowles jonathanknowles

Labels
None yet
Projects
None yet
Milestone
(ADP-313) Track Stakepool Retirements
Development

No branches or pull requests
2 participants
@jonathanknowles @Anviking