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.
| decodeRecordNamedT name getRecordSize decoder = | ||
| decodeListLikeT name decoder $ \result n -> | ||
| -- REVIEW: Is n really the requested/expected? It's the one we decode and | ||
| -- thus n is rather the actual/found. | ||
| lift $ matchSize ("Record " <> name) n (getRecordSize result) |
There was a problem hiding this comment.
Well, it was exactly my intention to reach a reviewer in this PR :)
bfce2c9 to
6662626
Compare
6662626 to
24c8a40
Compare
This depends on new package cardano-crypto-leios from cardano-base
24c8a40 to
5094471
Compare
d58a6be to
6d5c292
Compare
6d5c292 to
5a2478f
Compare
lehins
left a comment
There was a problem hiding this comment.
This is looking great. Thank you.
|
|
||
| -- cabal-allow-newer end | ||
| -- Points to cardano-base/ch1bo/cardano-crypto-leios | ||
| source-repository-package |
There was a problem hiding this comment.
Naturally, we can't merge this PR, until caradno-crypto-leios is on CHaP
There was a problem hiding this comment.
Sure, we need the upstream PR merged first and then I'll release it.
| then pure TNull | ||
| else genArrayTerm $ TInteger . toInteger <$> invalidIxIxs | ||
| txsTerm <- withAntiGen (withAnnotation "transactions") $ genArrayTerm txs | ||
| -- NOTE: This would not be a valid block because txs are not allowed when a |
There was a problem hiding this comment.
I'd like to understand all possible modes here for block body contents, so could you please provide a response to this message, so we make sure we get validation logic right.
This is my understanding today of valid block contents in Dijkstra:
- Praos block:
- Transactions, No Leios Certificate, maybe Peras Certificate
- No Transactions, No Leios Certificate, maybe Peras Certificate
- Ranking Block can contain:
- No Transactions, Leios Certificate, maybe Peras Certificate
- Endorsed Block:
- Transactions (many of them). No Leios Certificate, No Peras Certificate.
Is my understanding on block contents correct?
Also, I believe EB, must always follow RB, otherwise chain is invalid, right?
There was a problem hiding this comment.
From a Leios perspective we call all Praos blocks "Ranking blocks" (RB) and distinguish two cases for the block body:
- TxsRB = txs, no leios cert (and optional peras or other things)
- CertRB = no txs, leios cert (and optional peras or other things)
An "Endorser block" (EB) also has an invalid txs index, but otherwise just contains transaction hashes. One could inline those transactions and call the inlined structure the EB.
Yes, EBs would extend an RB (that announced the EB) and be followed by a CertRB (that certifies the EB).
It's currently not settled how we'd be validating EBs (by the ledger). A first sketch just folds calls to ApplyTx (mempool API) to update the ledger state when we see a CertRB. See this, very recent prototype code:
- Call to validate the EB txs (after loading them from disk and doing UTxO-HD) https://github.com/IntersectMBO/ouroboros-consensus/pull/2068/changes#diff-728ab9cd7036ec19e2f2c7d1836121ab14bdf1d3a4983238caab27042e8d3916R548
- This results in this call to the ledger here https://github.com/IntersectMBO/ouroboros-consensus/pull/2068/changes#diff-3a235edac184289e0b719564cdb5256ce2af015c8b34a4ce27c192b81220c888R177
|
|
||
| leios_signature = bytes .size 48 | ||
|
|
||
| peras_certificate = bytes |
There was a problem hiding this comment.
Is there any upper or lower limit on the size of certificate, aside from block body size limit?
There was a problem hiding this comment.
For leios (you commented on peras here), certs will be 48 bytes for the signature + committee size / 8 bytes + cbor overhead. The maximum committee size would be all registered pools ~ 3000 - typically it would rather be < 1000. So the maximum you can expect is 429B and typical 178B.
This follows the pattern of other cardano-base types.
Not sure if this is better, but the upstream module has a smaller API surface now.
72fe73e to
2e2eee2
Compare
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).