Skip to content

CDDL #420

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.

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

Open
wants to merge 7 commits into
base: cip/review
Choose a base branch
from
Open

CDDL #420

wants to merge 7 commits into from

Conversation

will-break-it
Copy link
Contributor

@will-break-it will-break-it commented Jun 19, 2025

  • Ranking Blocks
  • Votes & Certificates
  • Endorser Blocks
  • Input Blocks
  • Transactions

@will-break-it will-break-it requested review from bwbush and ch1bo June 19, 2025 12:11
, endorser_block_hash : hash32 ; Hash of the endorsed block (EB)
, persistent_voters : [* persistent_voter_id] ; Set of persistent voter IDs
, nonpersistent_voters : {* pool_id => bls_signature} ; Non-persistent voters with eligibility proofs
, ? aggregate_elig_sig : bls_signature ; Optional aggregate eligibility signature
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm wondering if "optional" is misleading: It is required if there are non-persistent voters. However, in some stake-distribution scenarios there might not be any non-persistent voters, in which case it is absent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about changing the comment to clarify its conditional existence based on the voter composition?

, ? aggregate_elig_sig   : bls_signature                           ; Aggregate eligibility signature (present when non-persistent voters exist)

Comment on lines +83 to +84
bls_signature = bytes .size 48 ; BLS12-381 G1 signature (compressed)
bls_vkey = bytes .size 96 ; BLS12-381 G2 public key (compressed)
Copy link
Collaborator

@bwbush bwbush Jun 19, 2025

Choose a reason for hiding this comment

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

My memory is that other parts of the Ledger use store BLS points uncompressed. I never understood why and I think that Leios should store them compressed. Decompressing them does take a bit of computation, however, because the y coordinate needs to be looked up on the curve. (Presumably a similar computation is needed for an uncompressed point, in order to ensure that it truly is on the curve.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to CIP-0381 it seems ok to use compressed and is recommended.

"even though uncompression is a lot more expensive per point than deserialisation, the size savings due to compression actually outweigh the speed gains due to serialisation because bytes per script are a lot more expensive than ExUnits per script in real terms."

Copy link
Collaborator

@bwbush bwbush left a comment

Choose a reason for hiding this comment

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

The RB and vote CDDLs looks good. I noted that the vote CDDL differs from an optimization that the simulators currently use. We'll have to discuss whether that optimization should be memorialized in the specification itself.

@bwbush
Copy link
Collaborator

bwbush commented Jun 19, 2025

For the IB and EB CDDL, we'll need to decide whether a node is allowed to produce multiple IBs in the same slot or multiple EBs in the same pipeline.

  • The simulations currently allow this.
  • The crypto benchmarks do not.
    We've flip-flopped on this a couple of times over the past six months.

Personally, I think the probability of being allowed multiple IBs or EBs is small enough (even for the biggest stakepools) that we should limit it to one. This simplifies the data structures, makes it easier to detect equivocation, etc.

@will-break-it
Copy link
Contributor Author

Personally, I think the probability of being allowed multiple IBs or EBs is small enough (even for the biggest stakepools) that we should limit it to one. This simplifies the data structures, makes it easier to detect equivocation, etc.

I agree. It would also simplify the network protocol presumably (by disallowing multiple announcements per slot).
I'll stick to a simple CDDL structure when I get to the point. Still looking at the base structure for EBs

@will-break-it will-break-it marked this pull request as ready for review June 20, 2025 06:36
@will-break-it will-break-it requested a review from nfrisby June 20, 2025 06:45
@will-break-it will-break-it self-assigned this Jun 20, 2025
@will-break-it will-break-it changed the title CDDL diffs CDDL Jun 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants