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

Support SlotNo <-> UTCTime conversions #1869

Closed
Anviking opened this issue Jul 7, 2020 · 1 comment
Closed

Support SlotNo <-> UTCTime conversions #1869

Anviking opened this issue Jul 7, 2020 · 1 comment
Assignees

Comments

@Anviking
Copy link
Collaborator

Anviking commented Jul 7, 2020

Context

⚠️ Ticket may be partly wrong. In the Shelley phase of Byron;Shelley, we have unlimited forecast range ⚠️

We currently have naive point -> time conversions that are going to break with the hard fork. We:

  • Show the time transactions were inserted to the ledger
  • Show the time of the next epoch
  • Show the time of future changes to the delegation status

To help us, there will be a query from consensus that will enable us to do time conversions locally. We will
/not/ have to send one request per conversion. But we will have to update the conversion-info
from time to time. (I would guess some time per epoch.)

For the wallet the most annoying thing will be that you'll need to update this from time to
time, so you'd need some kind of stateful abstraction that if a conversion is out of range,
asks for an updated interpreter, tries again; if still out of date, throw an exception (this
would then be a bug in the wallet, trying to look ahead too far).

All answers are certain and not subject to rollback.

You get one of two answers: "Yes, I can convert this thing, here's the result [and it is not
subject to rollback]" or ‚"No, we don't know that yet".

Times of future epochs

We can convert historial times. There is a forecast range. We can know the time of the next epoch
start, but not anything beyond it.

So this will still work:
Skärmavbild 2020-07-07 kl  11 19 11

But we can no longer provide epochStartTime for changes to the delegation:

data ApiEpochInfo = ApiEpochInfo
    { epochNumber :: !(ApiT EpochNo)
    , epochStartTime :: !UTCTime
    } deriving (Eq, Generic, Show)

Luckily it seems Daedalus doesn't use it:

Skärmavbild 2020-07-07 kl  11 19 15

Sync progress

We can no longer convert the current wall clock to a slot while syncing. We are currently relying on this for the sync progress calculation. Doing the opposite however: converting the genesis and wallet slots to times, and comparing with the current time, should work. But perhaps not as well on jormungandr where there may be a large gap of empty slots between block0 and block1.

Decision

  • Abstraction for point <-> UTCTime conversions for txs must be added Use SlotNo and TimeInterpreter #1901
  • nextEpoch :: ApiEpochInfo field in ApiNetworkInformation should be kept
  • Wallet delegation epochStartTime field should be removed
  • Wallet syncProgress should be changed to compare wallet time vs wall time instead of wallet slot vs wall slot Use SlotNo and TimeInterpreter #1901

Acceptance Criteria

  • ApiTransactions must show the correct insertedAt time
  • The nextEpoch time must still be availible for Daedalus
  • sync progress must not be broken

Development

QA

@Anviking Anviking self-assigned this Jul 7, 2020
@Anviking Anviking added this to Backlog in Adrestia via automation Jul 7, 2020
iohk-bors bot added a commit that referenced this issue Jul 8, 2020
1875: Move sync progress logic to new module r=Anviking a=Anviking

# Issue Number

#1869 

# Overview

- [x] Make the sync progress logic easier to grasp by moving it to its own module
- [x] Add a note about the calculation going to break in era-transitions


# Comments

- I believe we may have to have to provide _several_ methods of calculating the sync-progress. Maybe we need a different one for jormungandr. I think a new module is neat regardless.

<!-- Additional comments or screenshots to attach if any -->

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Johannes Lund <johannes.lund@iohk.io>
@Anviking Anviking moved this from Backlog to In Progress in Adrestia Jul 8, 2020
iohk-bors bot added a commit that referenced this issue Jul 8, 2020
1875: Move sync progress logic to new module r=Anviking a=Anviking

# Issue Number

#1869 

# Overview

- [x] Make the sync progress logic easier to grasp by moving it to its own module
- [x] Add a note about the calculation going to break in era-transitions


# Comments

- I believe we may have to have to provide _several_ methods of calculating the sync-progress. Maybe we need a different one for jormungandr. I think a new module is neat regardless.

<!-- Additional comments or screenshots to attach if any -->

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


1877: Fix new warnings in HLint v3.1.6 r=Anviking a=rvl

### Overview

HLint v3.1.6 can find more unused language extensions.

### Comments

CI version update in PR #1857.


Co-authored-by: Johannes Lund <johannes.lund@iohk.io>
Co-authored-by: Rodney Lorrimar <rodney.lorrimar@iohk.io>
iohk-bors bot added a commit that referenced this issue Jul 8, 2020
1875: Move sync progress logic to new module r=Anviking a=Anviking

# Issue Number

#1869 

# Overview

- [x] Make the sync progress logic easier to grasp by moving it to its own module
- [x] Add a note about the calculation going to break in era-transitions


# Comments

- I believe we may have to have to provide _several_ methods of calculating the sync-progress. Maybe we need a different one for jormungandr. I think a new module is neat regardless.

<!-- Additional comments or screenshots to attach if any -->

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Johannes Lund <johannes.lund@iohk.io>
iohk-bors bot added a commit that referenced this issue Jul 14, 2020
1890: Move slotting to separate module r=Anviking a=Anviking

# Issue Number

ADP-356 #1868, #1869 

# Overview


- [x] I have moved the existing slotting logic (`flatSlot`, `slotAt`, etc.) to a new module `Cardano.Wallet.Primitive.Slotting`
    - It is easier to keep track of remaining uses of static slotting this way
- [x] I started adding a ouroboros-consensus wrapper on top (currently unused)
    - [x] Added property `runQuery epochOf singleEraInterpreter == epochNumber . fromFlatSlot`


# Comments

- I left tests for slotting arithmetic in TypesSpec because the boundary between `Range` tests and slotting tests seemed unclear.
- Next step: Wrap all existing slotting logic using `Qry`/`Interpreter`.
- Could split into two PRs if you want

<!-- Additional comments or screenshots to attach if any -->

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Johannes Lund <johannes.lund@iohk.io>
iohk-bors bot added a commit that referenced this issue Jul 17, 2020
1901: Use SlotNo and TimeInterpreter r=Anviking a=Anviking

# Issue Number

ADP-356 #1868 #1869 


# Overview

- [x] Add TimeInterpreter, necessary queries, and a way to run them
Add tests comparing them with old implementation

- [x] Fundamentally change wallet to use SlotNo instead of SlotId
    - Patch together cardano-wallet-jormungandr to still work, despite the
binary using SlotId.

- [x] Remove redundant byron code

- [x] Sync progress calculation compares times now instead of slots

- [x] Rename the `slot` tables to `slot_no` to force automatic migration, preventing catastrophic re-interpretation of old data

# Comments

- Depends on #1890 

<!-- Additional comments or screenshots to attach if any -->

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Johannes Lund <johannes.lund@iohk.io>
iohk-bors bot added a commit that referenced this issue Jul 17, 2020
1901: Use SlotNo and TimeInterpreter r=Anviking a=Anviking

# Issue Number

ADP-356 #1868 #1869 


# Overview

- [x] Add TimeInterpreter, necessary queries, and a way to run them
Add tests comparing them with old implementation

- [x] Fundamentally change wallet to use SlotNo instead of SlotId
    - Patch together cardano-wallet-jormungandr to still work, despite the
binary using SlotId.

- [x] Remove redundant byron code

- [x] Sync progress calculation compares times now instead of slots

- [x] Rename the `slot` tables to `slot_no` to force automatic migration, preventing catastrophic re-interpretation of old data

# Comments

- Depends on #1890 

<!-- Additional comments or screenshots to attach if any -->

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Johannes Lund <johannes.lund@iohk.io>
iohk-bors bot added a commit that referenced this issue Jul 17, 2020
1901: Use SlotNo and TimeInterpreter r=Anviking a=Anviking

# Issue Number

ADP-356 #1868 #1869 


# Overview

- [x] Add TimeInterpreter, necessary queries, and a way to run them
Add tests comparing them with old implementation

- [x] Fundamentally change wallet to use SlotNo instead of SlotId
    - Patch together cardano-wallet-jormungandr to still work, despite the
binary using SlotId.

- [x] Remove redundant byron code

- [x] Sync progress calculation compares times now instead of slots

- [x] Rename the `slot` tables to `slot_no` to force automatic migration, preventing catastrophic re-interpretation of old data

# Comments

- Depends on #1890 

<!-- Additional comments or screenshots to attach if any -->

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Johannes Lund <johannes.lund@iohk.io>
iohk-bors bot added a commit that referenced this issue Jul 17, 2020
1901: Use SlotNo and TimeInterpreter r=Anviking a=Anviking

# Issue Number

ADP-356 #1868 #1869 


# Overview

- [x] Add TimeInterpreter, necessary queries, and a way to run them
Add tests comparing them with old implementation

- [x] Fundamentally change wallet to use SlotNo instead of SlotId
    - Patch together cardano-wallet-jormungandr to still work, despite the
binary using SlotId.

- [x] Remove redundant byron code

- [x] Sync progress calculation compares times now instead of slots

- [x] Rename the `slot` tables to `slot_no` to force automatic migration, preventing catastrophic re-interpretation of old data

# Comments

- Depends on #1890 

<!-- Additional comments or screenshots to attach if any -->

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Johannes Lund <johannes.lund@iohk.io>
iohk-bors bot added a commit that referenced this issue Jul 17, 2020
1901: Use SlotNo and TimeInterpreter r=Anviking a=Anviking

# Issue Number

ADP-356 #1868 #1869 


# Overview

- [x] Add TimeInterpreter, necessary queries, and a way to run them
Add tests comparing them with old implementation

- [x] Fundamentally change wallet to use SlotNo instead of SlotId
    - Patch together cardano-wallet-jormungandr to still work, despite the
binary using SlotId.

- [x] Remove redundant byron code

- [x] Sync progress calculation compares times now instead of slots

- [x] Rename the `slot` tables to `slot_no` to force automatic migration, preventing catastrophic re-interpretation of old data

# Comments

- Depends on #1890 

<!-- Additional comments or screenshots to attach if any -->

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Johannes Lund <johannes.lund@iohk.io>
@Anviking Anviking linked a pull request Jul 23, 2020 that will close this issue
6 tasks
@Anviking Anviking moved this from In Progress to QA in Adrestia Jul 28, 2020
@piotr-iohk
Copy link
Contributor

lgtm.

Adrestia automation moved this from QA to Closed Jul 29, 2020
iohk-bors bot added a commit that referenced this issue Jul 30, 2020
1977: Test getNetworkInformation handling of PastHorizonException r=Anviking a=Anviking

# Issue Number

#1869 /  #1960


# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- [x] Added a mock interpreter & network layer to produce a single unit test
- [x] I think we can make this a monadic property test easily


# Comments

<!-- Additional comments or screenshots to attach if any -->

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Johannes Lund <johannes.lund@iohk.io>
iohk-bors bot added a commit that referenced this issue Jul 30, 2020
1977: Test getNetworkInformation handling of PastHorizonException r=Anviking a=Anviking

# Issue Number

#1869 /  #1960


# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- [x] Added a mock interpreter & network layer to produce a single unit test
- [x] I think we can make this a monadic property test easily


# Comments

<!-- Additional comments or screenshots to attach if any -->

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Johannes Lund <johannes.lund@iohk.io>
iohk-bors bot added a commit that referenced this issue Jul 30, 2020
1977: Test getNetworkInformation handling of PastHorizonException r=Anviking a=Anviking

# Issue Number

#1869 /  #1960


# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- [x] Added a mock interpreter & network layer to produce a single unit test
- [x] I think we can make this a monadic property test easily


# Comments

<!-- Additional comments or screenshots to attach if any -->

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Johannes Lund <johannes.lund@iohk.io>
iohk-bors bot added a commit that referenced this issue Jul 30, 2020
1977: Test getNetworkInformation handling of PastHorizonException r=Anviking a=Anviking

# Issue Number

#1869 /  #1960


# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- [x] Added a mock interpreter & network layer to produce a single unit test
- [x] I think we can make this a monadic property test easily


# Comments

<!-- Additional comments or screenshots to attach if any -->

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Johannes Lund <johannes.lund@iohk.io>
iohk-bors bot added a commit that referenced this issue Jul 30, 2020
1977: Test getNetworkInformation handling of PastHorizonException r=Anviking a=Anviking

# Issue Number

#1869 /  #1960


# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- [x] Added a mock interpreter & network layer to produce a single unit test
- [x] I think we can make this a monadic property test easily


# Comments

<!-- Additional comments or screenshots to attach if any -->

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Johannes Lund <johannes.lund@iohk.io>
iohk-bors bot added a commit that referenced this issue Jul 31, 2020
1977: Test getNetworkInformation handling of PastHorizonException r=Anviking a=Anviking

# Issue Number

#1869 /  #1960


# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- [x] Added a mock interpreter & network layer to produce a single unit test
- [x] I think we can make this a monadic property test easily


# Comments

<!-- Additional comments or screenshots to attach if any -->

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Johannes Lund <johannes.lund@iohk.io>
@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
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants