Cleanup various things #411
Conversation
- New module `Test.Ouroboros.Consensus.Genesis.Setup` - Move `genChains` from its own module to `Genesis.Setup` - Move `TestSetup`-related code from `LongRangeAttack` to `Genesis.Setup`
Two bits of it still mattered: - `defaultCfg` moves to its own file `PeerSimulator.Config` - `ConnectionThread` moves to `PeerSimulator.Run`
- `PointSchedule` now contains a `NonEmpty Tick` which guarantees at least one tick and therefore access to the list of peers. - Get rid of fragments in both `AdvertisedPoints` and `PointSchedule`. - Cleanup imports of the `PointSchedule` module
| @@ -33,6 +44,7 @@ import Test.Util.TestBlock (TestBlock, TestBlockWith (tbSlot), | |||
| -- good: O─────1──2──3──4─────5──6──7 | |||
| -- bad: ╰─────3──4─────5 | |||
| -- | |||
| -- REVIEW: Generate more alternative chains? | |||
There was a problem hiding this comment.
We already implemented this in another changeset, to be reviewed in a call tomorrow
| data PointSchedule = | ||
| PointSchedule { ticks :: [Tick], frags :: Peers TestFrag } | ||
| newtype PointSchedule = | ||
| PointSchedule {ticks :: NonEmpty Tick} |
There was a problem hiding this comment.
Initially, we also required each individual peer to have nonempty schedules, but we weren't sure that it was required (also it was painful to work with).
Is there a reason we should choose either of those? An empty fork doesn't make any sense, but the chain generator probably prevents that anyway.
There was a problem hiding this comment.
Is there a reason we should choose either of those?
I don't understand the question yet. What would be the options under consideration? And what would be your preference?
There was a problem hiding this comment.
The change would be to disallow peer fragments that only consist of the genesis point. Right now that's accepted. The benefit might be that the point schedule generator would get nonempty chains as input, which could make some of the types easier, but maybe some of them more convoluted. I'm unsure what would be better.
There was a problem hiding this comment.
Thanks @Niols. The small decoupled commits made this PR rather easy to review.
| tests = testGroup "Genesis tests" | ||
| [ testProperty "blargh" prop_syncGenesis | ||
| ] | ||
| tests = testProperty "long range attack" prop_syncGenesis |
This PR only makes sense for the Genesis project.
This PR includes various cleanup operations. The commits are somewhat independent and they should speak for themselves. This PR should probably be rebased.