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

Chain DB: handle and test clock changes #759

Closed
mrBliss opened this issue Jul 15, 2019 · 11 comments · Fixed by #1554
Closed

Chain DB: handle and test clock changes #759

mrBliss opened this issue Jul 15, 2019 · 11 comments · Fixed by #1554
Assignees
@edsko @nfrisby
Labels
byron Required for a Byron mainnet: replace the old core nodes with cardano-node chain db consensus issues related to ouroboros-consensus daedalus When the wallet switches from cardano-sl to the new node testing
Milestone
S6 2020-02-13

Comments

@mrBliss
Copy link
Contributor

mrBliss commented Jul 15, 2019 •
edited by edsko
Loading

See https://github.com/input-output-hk/ouroboros-network/blob/master/ouroboros-consensus/docs/ChainDB.md#clock-changes
@mrBliss mrBliss added consensus issues related to ouroboros-consensus testnet labels Jul 15, 2019
@edsko edsko mentioned this issue Jul 15, 2019
Closed
@mrBliss mrBliss mentioned this issue Jul 15, 2019
Merged
@mrBliss mrBliss added the testing label Aug 2, 2019
@mrBliss mrBliss added this to the Consensus_Storage milestone Aug 2, 2019
@mrBliss
Copy link
Contributor Author

mrBliss commented Aug 5, 2019

Possible approach: start a thread that "watches" the current time and when it rolls back, shut down the node (and restart).

@mrBliss
Copy link
Contributor Author

mrBliss commented Aug 5, 2019

TODO: do we deal correctly with blocks (on-disk) from the future?

@mrBliss mrBliss mentioned this issue Oct 21, 2019
Closed
@edsko
Copy link
Contributor

edsko commented Nov 12, 2019

Moving this to daedalus since exchange and core nodes won't be changing their clocks.

@edsko edsko removed this from the release-node-1.0.0 testing milestone Nov 12, 2019
@edsko edsko added the daedalus When the wallet switches from cardano-sl to the new node label Nov 12, 2019
@edsko
Copy link
Contributor

edsko commented Nov 12, 2019

Marked as high priority because this is totally untested.

@mrBliss
Copy link
Contributor Author

mrBliss commented Dec 18, 2019

An NTP client will be running and notify the wallet when it is out of sync. Think about what we should do.

@mrBliss mrBliss added the byron Required for a Byron mainnet: replace the old core nodes with cardano-node label Jan 6, 2020
@mrBliss
Copy link
Contributor Author

mrBliss commented Jan 6, 2020

Current plan:

  • If time moved forward: waitUntilNextSlot will handle it correctly. We might briefly lag behind, but we'll correct it when waiting for the next slot.
  • If time moved backward: we can check in waitUntilNextSlot whether the computed next slot number is actually the next one. If it is not, time must have moved back. We could allow staying on the same slot number, but we certainly can't allow going back a slot, we should throw an exception, shutting down the node.

Note that we (the node) don't have to run an NTP client for this. We are using getCurrentTime (wall clock) in waitUntilNextSlot.

@mrBliss mrBliss added this to the S5 2020-01-30 milestone Jan 7, 2020
@mrBliss
Copy link
Contributor Author

mrBliss commented Jan 7, 2020

Implementation: @edsko
(Protocol) testing: @nfrisby

@mrBliss mrBliss modified the milestones: S5 2020-01-30, S6 2020-02-13 Jan 24, 2020
This was referenced Jan 31, 2020
Merged
Open
@mrBliss
Copy link
Contributor Author

mrBliss commented Feb 6, 2020

The implementation was completed in #1554. We have created a new ticket for testing it: IntersectMBO/ouroboros-consensus#675.

@mrBliss mrBliss closed this as completed Feb 6, 2020
@edsko
Copy link
Contributor

edsko commented Feb 7, 2020

Re-opening as this isn't merged yet due to dependency on #1539 . Since it's only the tests that depend on that, though, will submit two separate PRs.

@edsko
Copy link
Contributor

edsko commented Feb 7, 2020 •
edited
Loading

We have created a new ticket for testing it: IntersectMBO/ouroboros-consensus#675.

Mind that it does come with tests, it just doesn't do consensus-level tests.

iohk-bors bot added a commit that referenced this issue Feb 7, 2020
@iohk-bors @edsko
Merge #1554
901b746 
1554: Detect clock changes r=edsko a=edsko

Closes #759.

Code is tested using mock time; result of the test labelling:

```
# cabal run test-consensus -- -p delayClockShift --quickcheck-replay=680184 --quickcheck-tests 10000
Up to date
ouroboros-consensus
  WallClock
    delayClockShift: OK (18.51s)
      +++ OK, passed 10000 tests.
      
      schedule goes back (10000 in total):
      63.07% False
      36.93% True
      
      schedule length (10000 in total):
      81.03% R_Gt 20
       9.26% R_Btwn (10,20)
       4.08% R_Btwn (5,10)
       1.04% R_Btwn (4,5)
       1.03% R_Eq 1
       0.96% R_Eq 2
       0.94% R_Eq 4
       0.87% R_Eq 0
       0.79% R_Eq 3
      
      schedule skips (10000 in total):
      38.66% R_Btwn (10,20)
      24.96% R_Btwn (5,10)
       5.59% R_Eq 0
       5.43% R_Eq 2
       5.27% R_Btwn (4,5)
       5.12% R_Eq 1
       5.08% R_Eq 3
       4.99% R_Eq 4
       4.90% R_Gt 20

All 1 tests passed (18.52s)
```

I also verified that the exception bubbles up to the node. Ran the latest node with this PR, set my clock back an hour, and got

```
cardano-node: ExceptionInLinkedThread "ThreadId 20" (SystemClockMovedBack 2020-01-31 11:15:31.00184141 UTC (SlotNo {unSlotNo = 3713312}) (SlotNo {unSlotNo = 3713132}))
```

There is no need to define a custom error policy for this due to #1553 , nor a custom exit failure due to #1551 (comment) .

Co-authored-by: Edsko de Vries <[email protected]>
@edsko
Copy link
Contributor

edsko commented Feb 7, 2020

Closed as of #1554 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@edsko edsko

@nfrisby nfrisby

Labels
byron Required for a Byron mainnet: replace the old core nodes with cardano-node chain db consensus issues related to ouroboros-consensus daedalus When the wallet switches from cardano-sl to the new node testing
Projects
None yet
Milestone
S6 2020-02-13
Development

Successfully merging a pull request may close this issue.

Detect clock changes IntersectMBO/ouroboros-network
3 participants
@mrBliss @edsko @nfrisby