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

add a more coherent (less random) block generator #2280

Merged
merged 1 commit into from May 12, 2021
Merged

add a more coherent (less random) block generator #2280

merged 1 commit into from May 12, 2021

Conversation

JaredCorduan
Copy link
Contributor

In 7447dd7 we changed genBlock to
produce blocks with more noise. This turns out to have broken some tests
in the consensus layer that relied on some coherency in the block.

See IntersectMBO/ouroboros-network#3134

To fix this, we add a new block generator genCoherentBlock, which is
almost exactly like the previous genBlock, but does not require the
PreAlonzo constraint.

The only difference (besides constraints) between the previous genBlock and genCoherentBlock is replacing

txs <- listOf (genTx @era)

with

txs <- arbitrary

@JaredCorduan JaredCorduan requested a review from nfrisby May 12, 2021 17:28
nfrisby
nfrisby approved these changes May 12, 2021
View reviewed changes
Copy link
Contributor

@nfrisby nfrisby left a comment

Choose a reason for hiding this comment

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

I integrated this into my ouroboros-network branch and now the tests are passing again! 👍

Thank you for the rapid response!

@nfrisby nfrisby mentioned this pull request May 12, 2021
Closed
add a more coherent (less random) block generator
a50b834 
In 7447dd7 we changed genBlock to
produce blocks with more noise. This turns out to have broken some tests
in the consensus layer that relied on some coherency in the block.

See IntersectMBO/ouroboros-network#3134

To fix this, we add a new block generator genCoherentBlock, which is
almost exactly like the previous genBlock, but does not require the
PreAlonzo constraint.
@JaredCorduan JaredCorduan merged commit 22f53f7 into master May 12, 2021
@iohk-bors iohk-bors bot deleted the jc/genCoherentBlock branch May 12, 2021 20:17
nfrisby added a commit to IntersectMBO/ouroboros-network that referenced this pull request May 13, 2021
@nfrisby
cabal.project and sources.json: update cardano-ledger-specs dependency
f89d0d5 
This commit hash matches and the sources.json content matches-or-exceeds that
of the first cardano-ledger-specs commit that provides everything we require
for our initial Alonzo integration (ie no serialization tests, but a
Mary-to-Alonzo HF ThreadNet test).

We needed to introduce the `Coherent` QuickCheck modifier because PR
IntersectMBO/cardano-ledger#2274 loosened the `Arbitrary` instance for
Shelley blocks. We have a few tests that assumed the arbitrary blocks would
pass "integrity" checks and that the header's claim for the block body's size
was mostly correct. The follow-up IntersectMBO/cardano-ledger#2280
re-introduced the generator for coherent header-body pairs, and the new
`Coherent` modifier lets us select that for the requisite properties.
nfrisby added a commit to IntersectMBO/ouroboros-network that referenced this pull request May 13, 2021
@nfrisby
cabal.project and sources.json: update cardano-ledger-specs dependency
b29f6e1 
This commit hash matches and the sources.json content matches-or-exceeds that
of the first cardano-ledger-specs commit that provides everything we require
for our initial Alonzo integration (ie no serialization tests, but a
Mary-to-Alonzo HF ThreadNet test).

We needed to introduce the `Coherent` QuickCheck modifier because PR
IntersectMBO/cardano-ledger#2274 loosened the `Arbitrary` instance for
Shelley blocks. We have a few tests that assumed the arbitrary blocks would
pass "integrity" checks and that the header's claim for the block body's size
was mostly correct. The follow-up IntersectMBO/cardano-ledger#2280
re-introduced the generator for coherent header-body pairs, and the new
`Coherent` modifier lets us select that for the requisite properties.
nfrisby added a commit to IntersectMBO/ouroboros-network that referenced this pull request May 13, 2021
@nfrisby
cabal.project and sources.json: update cardano-ledger-specs dependency
6233e44 
This commit hash matches and the sources.json content matches-or-exceeds that
of the first cardano-ledger-specs commit that provides everything we require
for our initial Alonzo integration (ie no serialization tests, but a
Mary-to-Alonzo HF ThreadNet test).

We needed to introduce the `Coherent` QuickCheck modifier because PR
IntersectMBO/cardano-ledger#2274 loosened the `Arbitrary` instance for
Shelley blocks. We have a few tests that assumed the arbitrary blocks would
pass "integrity" checks and that the header's claim for the block body's size
was mostly correct. The follow-up IntersectMBO/cardano-ledger#2280
re-introduced the generator for coherent header-body pairs, and the new
`Coherent` modifier lets us select that for the requisite properties.
nfrisby added a commit to IntersectMBO/ouroboros-network that referenced this pull request May 13, 2021
@nfrisby
cabal.project and sources.json: update cardano-ledger-specs dependency
b117726 
This commit hash matches and the sources.json content matches-or-exceeds that
of the first cardano-ledger-specs commit that provides everything we require
for our initial Alonzo integration (ie no serialization tests, but a
Mary-to-Alonzo HF ThreadNet test).

We needed to introduce the `Coherent` QuickCheck modifier because PR
IntersectMBO/cardano-ledger#2274 loosened the `Arbitrary` instance for
Shelley blocks. We have a few tests that assumed the arbitrary blocks would
pass "integrity" checks and that the header's claim for the block body's size
was mostly correct. The follow-up IntersectMBO/cardano-ledger#2280
re-introduced the generator for coherent header-body pairs, and the new
`Coherent` modifier lets us select that for the requisite properties.
nfrisby added a commit to IntersectMBO/ouroboros-network that referenced this pull request May 13, 2021
@nfrisby
cabal.project and sources.json: update cardano-ledger-specs dependency
1d5b0c7 
This commit hash matches and the sources.json content matches-or-exceeds that
of the first cardano-ledger-specs commit that provides everything we require
for our initial Alonzo integration (ie no serialization tests, but a
Mary-to-Alonzo HF ThreadNet test).

We needed to introduce the `Coherent` QuickCheck modifier because PR
IntersectMBO/cardano-ledger#2274 loosened the `Arbitrary` instance for
Shelley blocks. We have a few tests that assumed the arbitrary blocks would
pass "integrity" checks and that the header's claim for the block body's size
was mostly correct. The follow-up IntersectMBO/cardano-ledger#2280
re-introduced the generator for coherent header-body pairs, and the new
`Coherent` modifier lets us select that for the requisite properties.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nfrisby nfrisby nfrisby approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@JaredCorduan @nfrisby