Test & fix backwards compatibility between Byron and Cardano #2385
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sometimes it is more convenient to simply define the data/type instances without having to define the whole type class instances. For example, when wrapping an existing block to override the serialisation instances, we should not have to provide instances for classes like `LedgerSupportsMempool` when we only want to wrap the associated (data) types.
Sometimes it is more convenient to simply define the data/type instances without having to define the whole type class instances. For example, when wrapping an existing block to override the serialisation instances, we should not have to provide instances for classes like `GetHeader` when we only want to wrap the associated data type.
Sometimes it is more convenient to simply define the data/type instances without having to define the whole type class instances. For example, when wrapping an existing block to override the serialisation instances, we should not have to provide instances for classes like `QueryLedger` when we only want to wrap the associated data type.
These pattern synonyms could be used to construct queries, but not to pattern match on them because it is a GADT. Note the double "double" arrow, the second set of constraints are those provided by the pattern.
mrBliss
commented
Jul 10, 2020
Comment on lines
+163
to
+162
| instance ConvertRawHash ByronToCardano where | ||
| toRawHash _ = toRawHash pb | ||
| fromRawHash _ = fromRawHash pb |
There was a problem hiding this comment.
Yes, a superclass constraint of SerialiseNodeToNodeConstraints.
ouroboros-consensus-cardano/test/Test/Consensus/Cardano/ByronCompatibility.hs
Show resolved
Hide resolved
ouroboros-consensus-cardano/test/Test/Consensus/Cardano/ByronCompatibility.hs
Outdated
Show resolved
Hide resolved
ouroboros-consensus-cardano/test/Test/Consensus/Cardano/ByronCompatibility.hs
Outdated
Show resolved
Hide resolved
ouroboros-consensus-cardano/test/Test/Consensus/Cardano/ByronCompatibility.hs
Outdated
Show resolved
Hide resolved
ouroboros-consensus-cardano/test/Test/Consensus/Cardano/ByronCompatibility.hs
Outdated
Show resolved
Hide resolved
ouroboros-consensus-cardano/test/Test/Consensus/Cardano/ByronCompatibility.hs
Outdated
Show resolved
Hide resolved
edsko
approved these changes
Jul 10, 2020
Some idiot broke backwards compatibility in #2349. We were no longer able to exchange queries via the `LocalStateQuery` protocol with a Byron-only node.
mrBliss
force-pushed
the
mrBliss/backwards-compat
branch
from
ce9f147
to
8929cb9
Compare
|
bors merge |
iohk-bors bot
added a commit
that referenced
this pull request
Jul 10, 2020
2385: Test & fix backwards compatibility between Byron and Cardano r=mrBliss a=mrBliss Test backwards compatibility between Byron and Cardano Fixes #2361. A node running `CardanoBlock` can communicate with older nodes that run `ByronBlock`, until the hard fork happens (as the other nodes won't support Shelley). Test that we're actually backwards compatible. This test did exactly what it was supposed to do: it found a bug! Some idiot broke backwards compatibility in #2349. We were no longer able to exchange queries via the `LocalStateQuery` protocol with a Byron-only node. Fix it. Co-authored-by: Thomas Winant <thomas@well-typed.com>
Build failed |
Fixes #2361. A node running `CardanoBlock` can communicate with older nodes that run `ByronBlock`, until the hard fork happens (as the other nodes won't support Shelley). Test that we're actually backwards compatible.
mrBliss
force-pushed
the
mrBliss/backwards-compat
branch
from
8929cb9
to
1a53abe
Compare
|
bors merge |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Test backwards compatibility between Byron and Cardano
Fixes #2361.
A node running
CardanoBlockcan communicate with older nodes that runByronBlock, until the hard fork happens (as the other nodes won't supportShelley). Test that we're actually backwards compatible.
This test did exactly what it was supposed to do: it found a bug!
Some idiot broke backwards compatibility in #2349. We were no longer able to exchange queries via the
LocalStateQueryprotocolwith a Byron-only node. Fix it.