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

Use the IO instance of MonadRandom #1677

Merged
merged 1 commit into from
Feb 20, 2020
Merged

Use the IO instance of MonadRandom #1677

merged 1 commit into from
Feb 20, 2020

Conversation

mrBliss
Copy link
Contributor

@mrBliss mrBliss commented Feb 20, 2020

Fixes #1616.

Previously, block production had to live in STM, but this is no longer the
case as of #1445. This means that we no longer need to run MonadRandom
computations in STM. To do that, we stored a DRG in a TVar.

Now, we can simply use IO's instance of MonadRandom. In the tests we still
use the DRG-in-a-TVar trick, but we split it whenever we get a DRG.

We use the RunMonadRandom record for this purpose.


Replace NodeState with MonadState

Previously, we need a separate HasNodeState class and NodeStateT monad
transformer because there was already a StateT in our stack, i.e., the one
containing the DRG. As that is gone, we can switch back to a regular
MonadState + StateT.

The cost is an orphan instance: MonadRandom (StateT s m)

@mrBliss mrBliss added the consensus issues related to ouroboros-consensus label Feb 20, 2020
@mrBliss mrBliss requested a review from edsko February 20, 2020 10:23
Copy link
Contributor

@edsko edsko left a comment

Choose a reason for hiding this comment

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

Thank god somebody who actually knows what they're doing took over this PR 🙄

Previously, block production had to live in STM, but this is no longer
the case as of #1445. This means that we no longer need to run
`MonadRandom` computations in STM. To do that, we stored a DRG in a
TVar.  Now, we can simply use IO's instance of `MonadRandom`. In the
tests we still use the DRG-in-a-TVar trick, but we split it whenever we
get a DRG.  We use the `RunMonadRandom` record for this purpose.

In addition, this removes all notion of state from `ConsensusProtocol`,
and instead makes this part of the `RunNode` interface.

This also fixes and re-enables the praos tests.

Closes #1616
Closes #1678
Closes #1545

Co-authored-by: Thomas Winant <thomas@well-typed.com>
@edsko
Copy link
Contributor

edsko commented Feb 20, 2020

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Feb 20, 2020

@iohk-bors iohk-bors bot merged commit e31d8e7 into master Feb 20, 2020
@iohk-bors iohk-bors bot deleted the mrBliss/monadrandom branch February 20, 2020 15:26
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Don't run block production in STM
2 participants