Add Dijkstra ledger era #1567
Add Dijkstra ledger era #1567
Conversation
geo2a
force-pushed
the
geo2a/issue1544-dijkstra-era
branch
2 times, most recently
from
17f9373 to
c9dd81e
Compare
geo2a
force-pushed
the
geo2a/issue1544-dijkstra-era
branch
2 times, most recently
from
dcb8014 to
77f0949
Compare
| -- TODO(geo2a): feels weird to introduce a type alias and immediately deprecate it. | ||
| -- Do we still need that? | ||
| type StandardDijkstra = DijkstraEra |
There was a problem hiding this comment.
This can probably be removed, and all the others too.
geo2a
force-pushed
the
geo2a/issue1544-dijkstra-era
branch
11 times, most recently
from
d17b50a to
1cf12d9
Compare
geo2a
force-pushed
the
geo2a/issue1544-dijkstra-era
branch
4 times, most recently
from
ee53699 to
eabfb42
Compare
geo2a
requested review from
nfrisby,
amesgen,
fraser-iohk and
dnadales
as code owners
ouroboros-consensus-cardano/src/shelley/Ouroboros/Consensus/Shelley/Eras.hs
Outdated
Show resolved
Hide resolved
...consensus-cardano/src/ouroboros-consensus-cardano/Ouroboros/Consensus/Cardano/CanHardFork.hs
Outdated
Show resolved
Hide resolved
ouroboros-consensus-cardano/test/cardano-test/Test/Consensus/Cardano/Translation.hs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
For consistency with db-synthesizer, you could also read the (trivial) Dijkstra genesis file in here.
(In general, it would be nice to avoid duplicating this logic, also see #1072:
The ideal path to this solution is to separate out the parts of cardano-node and ouroboros-consensus that read node configuration in favor of a shared component. This will prevent any future divergence.
Yak shaving...)
|
Update: this was resolved by additional fix-up for the CDDL spec. I've added a relevant note to I'm hitting a bit of a wall with my attempts to update the NodeToNode cddls to be aware of the Dijkstra era. I'm parking this PR for now until I've figured out how to address the failing tests. The following command will trigger the failing test: cabal test -O0 cardano-test --test-options '-p "Dijkstra.CDDL"'Specifically, the tests for ls -la failing_cddl_tests/
total 176
drwxr-xr-x 1 geo2a users 340 Jul 10 14:03 .
drwxr-xr-x 1 geo2a users 254 Jul 10 14:03 ..
-rw-r--r-- 1 geo2a users 325 Jul 10 14:03 call_cuddle_serialisedCardanoBlock_failing.sh
-rw-r--r-- 1 geo2a users 265 Jul 10 14:03 call_cuddle_tx_failing.sh
-rw------- 1 geo2a users 1752 Jul 10 14:03 serialisedCardanoBlock_failing.cbor
-rw-r--r-- 1 geo2a users 79115 Jul 10 14:03 serialisedCardanoBlock_failing.cddl
-rw------- 1 geo2a users 899 Jul 10 14:03 tx_failing.cbor
-rw-r--r-- 1 geo2a users 80987 Jul 10 14:03 tx_failing.cddl |
geo2a
force-pushed
the
geo2a/issue1544-dijkstra-era
branch
2 times, most recently
from
5edc875 to
30a924e
Compare
geo2a
force-pushed
the
geo2a/issue1544-dijkstra-era
branch
from
30a924e to
86caea9
Compare
| -- | The hard fork enabled, and the Shelley, Allegra, Mary, Alonzo and Babbage | ||
| -- and Conway eras enabled, using 'ShelleyNodeToClientVersion8' for the | ||
| -- | The hard fork enabled, and the Shelley, Allegra, Mary, Alonzo, Babbage, | ||
| -- Conway and Dijkstra eras enabled, using 'ShelleyNodeToClientVersion8' for the | ||
| -- Shelley-based eras. | ||
| pattern CardanoNodeToClientVersion12 :: BlockNodeToClientVersion (CardanoBlock c) |
There was a problem hiding this comment.
I suspect many of these old NTC versions could be dropped.
There was a problem hiding this comment.
How do I know which one of them are not needed anymore? Do I need to look somewhere in cardano-node or ouroborus-network?
| @@ -833,6 +897,7 @@ protocolInfoCardano paramsCardano | |||
| (Shelley.ShelleyStorageConfig tpraosSlotsPerKESPeriod k) | |||
| (Shelley.ShelleyStorageConfig tpraosSlotsPerKESPeriod k) | |||
| (Shelley.ShelleyStorageConfig tpraosSlotsPerKESPeriod k) | |||
| (Shelley.ShelleyStorageConfig tpraosSlotsPerKESPeriod k) | |||
There was a problem hiding this comment.
Ah I didn't realize this was called tpraos even for the praos blocks.
There was a problem hiding this comment.
Hmmmm, we also have praosSlotsPerKESPeriod in scope, which is defined as SL.sgSlotsPerKESPeriod genesisShelley. In turn, tpraosSlotsPerKESPeriod is defined through a couple more indirection to be the same thing.
So while the values do coincide, it is probably morally right to use tpraosSlotsPerKESPeriod for Praos era blocks.
ouroboros-consensus-cardano/src/shelley/Ouroboros/Consensus/Shelley/Eras.hs
Show resolved
Hide resolved
| ShelleyCompatible p DijkstraEra => | ||
| TxLimits (ShelleyBlock p DijkstraEra) | ||
| where | ||
| type TxMeasure (ShelleyBlock p DijkstraEra) = ConwayMeasure |
There was a problem hiding this comment.
Shouldn't we have a DijkstraMeasure that might be a newtype over ConwayMeasure? I guess it is not really necessary though
| @@ -178,7 +182,7 @@ fromShelleyLedgerExamples | |||
| results = | |||
| labelled | |||
| [ ("LedgerTip", SomeResult GetLedgerTip (blockPoint blk)) | |||
| , ("EpochNo", SomeResult GetEpochNo 10) | |||
| , ("EpochNo", SomeResult GetEpochNo (EpochNo 10)) | |||
There was a problem hiding this comment.
EpochNo is no longer an instance of Num?
geo2a
force-pushed
the
geo2a/issue1544-dijkstra-era
branch
from
86caea9 to
f5a1f86
Compare
- update package names - restructure headings - add section on `Ouroboros.Consensus.Cardano.Ledger`
…n17` - update golden files for `Result_*_MaxMajorProtocolVersion`
geo2a
force-pushed
the
geo2a/issue1544-dijkstra-era
branch
from
9772e8b to
a8f0463
Compare
geo2a
force-pushed
the
geo2a/issue1544-dijkstra-era
branch
2 times, most recently
from
1be4de3 to
6c2b2e1
Compare
geo2a
force-pushed
the
geo2a/issue1544-dijkstra-era
branch
from
6c2b2e1 to
91eb2fd
Compare
Fixes #1544
Needs IntersectMBO/cardano-ledger#5136--- the Ledger pr was merged tomaster; I've updates the s-r-p.Needs draft integration withI've attempted to integrate withcardano-nodecardano-node, but abandoned this effort, as we first need an integration intocardano-api.