Add LeiosCert to DijkstraBlockBody#5872
Conversation
There was a problem hiding this comment.
Pull request overview
This PR extends the Dijkstra era block body format to optionally include a Leios certificate (LeiosCert) and updates the corresponding CBOR and CDDL representations, along with test generators/specs to exercise both the “TxsRB” and “CertRB” shapes.
Changes:
- Add
StrictMaybe LeiosCerttoDijkstraBlockBodyRaw/DijkstraBlockBodyand update CBOR encoding/decoding accordingly. - Regenerate/update Dijkstra CDDL + Huddle rules to include
leios_certificate(and make cert fields/ nilinblock_body). - Update testlib instances/generators and CDDL specs to cover both block body shapes; add new
cardano-crypto-leiosdependency wiring.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| libs/cardano-ledger-binary/src/Cardano/Ledger/Binary/Decoding/Decoder.hs | Adds an in-code review note inside decodeRecordNamedT (should be removed). |
| eras/dijkstra/impl/testlib/Test/Cardano/Ledger/Dijkstra/TreeDiff.hs | Adds ToExpr LeiosCert for better diffs without orphan signature instances. |
| eras/dijkstra/impl/testlib/Test/Cardano/Ledger/Dijkstra/Binary/Annotator.hs | Updates block body decoding to expect 4 fields and decode optional Leios cert. |
| eras/dijkstra/impl/testlib/Test/Cardano/Ledger/Dijkstra/Arbitrary.hs | Adds Arbitrary LeiosCert and new small block body generators for “TxsRB” vs “CertRB”. |
| eras/dijkstra/impl/test/Test/Cardano/Ledger/Dijkstra/Binary/CddlSpec.hs | Splits block_body CDDL round-trip specs into “TxsRB” and “CertRB”. |
| eras/dijkstra/impl/src/Cardano/Ledger/Dijkstra/BlockBody/Internal.hs | Adds Leios cert field + updates CBOR encode/decode; updates EncCBORGroup (currently mismatched for StrictMaybe encoding). |
| eras/dijkstra/impl/cddl/lib/Cardano/Ledger/Dijkstra/HuddleSpec.hs | Adds Leios certificate/signature rules; updates block_body rule layout and generator to 4 fields. |
| eras/dijkstra/impl/cddl/data/dijkstra.cddl | Updates block_body CDDL and defines leios_certificate / leios_signature. |
| eras/dijkstra/impl/cardano-ledger-dijkstra.cabal | Adds cardano-crypto-leios (+ hedgehog-quickcheck for generators) to relevant components. |
| cabal.project | Adds a source-repository-package pin to pull in cardano-crypto-leios from cardano-base. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
bfce2c9 to
6662626
Compare
24c8a40 to
5094471
Compare
6d5c292 to
5a2478f
Compare
lehins
left a comment
There was a problem hiding this comment.
This is looking great. Thank you.
72fe73e to
2e2eee2
Compare
ce21132 to
a027cb9
Compare
This depends on new package cardano-crypto-leios from cardano-base
This follows the pattern of other cardano-base types.
55b8e91 to
6d22a5d
Compare
Not sure if this is better, but the upstream module has a smaller API surface now.
612c1ce to
b1ca9bf
Compare
b1ca9bf to
4543e08
Compare
Also fix hlint issues in the module
|
These mutation tests are really cool! https://github.com/IntersectMBO/cardano-ledger/actions/runs/28398403686/job/84150174066#step:9:1 But it means that I need change or at least deprecate the current cert encoder already 😅😅 |
8618cd2 to
7b69ca5
Compare
Replaces mocked certificates with what was specified in CIP-164. Depends on a new `cardano-crypto-leios` package that only holds the `LeiosCert` IntersectMBO/cardano-base#670 and the cardano-ledger which integrates that into the `DijkstraBlockBody` IntersectMBO/cardano-ledger#5872 Explores code paths of certificate validation in the `applyBlock` / block validation part of consensus. Aggregates votes whenever added to the `LeiosVoteState` and creates a certificate when exceeding the (hard-coded) threshold. The forge loop gets the certificate from the `LeiosVoteState` and uses it to decide whether we can build a `CertRB`. TODO: - [ ] Must not use committee of different epoch when creating block. Either forecast (and re-aggregate the cert) or just index certificates in the vote state by epoch and query using the current one. - Left a FIXME in `VoteState.addVote` which will by extension affect `queryCert` - [x] Should clean up the many type class methods - maybe anticipating an upcoming `applyBlock :: ... -> m (Either (LedgerErr l) (LedgerResult l (l DiffMK))` - [ ] Replace hacky integration with merges from `master` branches once the `11.1` work was integrated (= Blocked by `11.1` release now.. or we do that cleanup later) - [x] Use integrated `master` for `ouroboros-network` - [x] Use integrated `master` for `cardano-ledger` - [ ] Use integrated `master` for `ouroboros-consensus`
Description
This adds the
LeiosCertas anSMaybeto theDijkstraBlockBody. The type and its encoding is defined incardano-crypto-leios, a new package on thecardano-baserepository. See IntersectMBO/cardano-base#670This also includes a fix of the
DijkstrBlockBodyRawinstance forEncCBORGroupinstance: it was missing the invalid transactions field.New
block_bodyCDDL after this change (also regenerated in the diff):with
Checklist
CHANGELOG.mdfiles updated for packages with externally visible changes.NOTE: New section is never added with the code changes. (See RELEASING.md).
.cabalandCHANGELOG.mdfiles when necessary, according to theversioning process.
.cabalfiles updated when necessary.NOTE: If bounds change in a cabal file, that package itself must have a version increase. (See RELEASING.md).
scripts/fourmolize.sh).scripts/cabal-format.sh).scripts/gen-cddl.sh)hie.yamlupdated (usescripts/gen-hie.sh).