Rename epoch to slot in XMSS signature API to align with leanSpec#184
Rename epoch to slot in XMSS signature API to align with leanSpec#184MegaRedHand merged 3 commits intomainfrom
Conversation
The parameter named epoch actually receives a slot number everywhere. The leanSpec renamed this parameter from epoch to slot to reflect what it actually is — leanConsensus has no epoch concept unlike the Beacon Chain. Pure rename across production code, docs, and tests. No behavioral change.
🤖 Kimi Code ReviewReview SummaryThis PR correctly fixes a naming inconsistency by replacing Issues Found1. Critical: Potential panic in production code (Multiple locations)
Fix: let slot: u32 = attestation.data.slot.try_into()
.map_err(|_| StoreError::InvalidSlot(attestation.data.slot))?;2. Minor: Incomplete variable rename (Line 1176)
Fix: let slot: u32 = attestation.data.slot.try_into().expect("slot exceeds u32");3. Documentation consistency
Fix: Update comments to consistently use "slot" terminology. Security Assessment
Performance Impact
OverallThe PR correctly addresses the naming inconsistency. The only blocking issue is the use of Automated review by Kimi (Moonshot AI) · custom prompt |
🤖 Codex Code Review
A malformed/high slot can panic the process. Suggested change: map conversion failures to Overall, the PR is mostly a semantic rename ( Automated review by OpenAI Codex · custom prompt |
🤖 Claude Code ReviewPR 184 Review: Rename
|
Greptile SummaryThis PR is a pure naming alignment refactor, renaming the Key observations:
Confidence Score: 4/5
|
| Filename | Overview |
|---|---|
| crates/common/types/src/signature.rs | Pure parameter rename of epoch → slot in ValidatorSignature::is_valid (line 44) and ValidatorSecretKey::sign (line 88). No logic changes. Clean and complete. |
| crates/common/crypto/src/lib.rs | Rename is complete in production API (aggregate_signatures line 83, verify_aggregated_signature line 139) and most tests, but the test helper generate_keypair_and_sign retains the signing_epoch parameter name (lines 179, 190), creating a minor inconsistency with all its call sites that now pass a slot variable. |
| crates/blockchain/src/key_manager.rs | Rename is complete and consistent across sign_attestation (line 75), sign_message (lines 84-95), and internal call chain (line 104). No issues. |
| crates/blockchain/src/store.rs | All four epoch: u32 local variables renamed to slot: u32 at their respective call sites (line 172, 375-378, 440-446, 1163-1180). All production verification call sites use the renamed slot parameter correctly. Clean. |
Sequence Diagram
sequenceDiagram
participant Store as store.rs
participant KM as key_manager.rs
participant Crypto as crypto/lib.rs
participant Sig as signature.rs
participant Leansig as leansig crate
Note over Store,Leansig: Signing flow
KM->>KM: sign_attestation(validator_id, attestation_data)
KM->>KM: slot = attestation_data.slot as u32
KM->>Sig: secret_key.sign(slot, message_hash)
Sig->>Leansig: LeanSignatureScheme::sign(&sk, slot, message)
Leansig-->>Sig: LeanSigSignature
Sig-->>KM: ValidatorSignature
KM-->>Store: XmssSignature
Note over Store,Leansig: Individual verification flow
Store->>Store: slot = attestation.data.slot.try_into()
Store->>Sig: signature.is_valid(&pubkey, slot, &data_root)
Sig->>Leansig: LeanSignatureScheme::verify(&pk, slot, message, &sig)
Leansig-->>Sig: bool
Sig-->>Store: bool
Note over Store,Leansig: Aggregated verification flow
Store->>Crypto: verify_aggregated_signature(&proof, pubkeys, &data_root, slot)
Crypto->>Leansig: xmss_verify_aggregated_signatures(&pks, message, &aggregate, slot)
Leansig-->>Crypto: Result
Crypto-->>Store: Result
Comments Outside Diff (1)
-
crates/common/crypto/src/lib.rs, line 179-190 (link)Incomplete rename in test helper
The
generate_keypair_and_signhelper's parameter is still namedsigning_epoch(line 179) and is used as-is on line 190. However, all call sites now pass a variable namedslot(e.g., lines 218, 252, 279, 301), creating a mismatch in terminology.Since the entire purpose of this PR is to rename
epoch→slotfor consistency, consider renaming this parameter as well:And on line 190:
This ensures the test helper's signature is consistent with the renamed API terminology throughout the codebase.
Last reviewed commit: cfd6160
Motivation
The leanSpec reference implementation renamed the XMSS signing/verification parameter from
epochtoslot(commit3b9d054, part of PR #412).In ethlambda, the actual value passed to this parameter is always a slot number (e.g.,
attestation.data.slot as u32), so theepochnaming was misleading — in Ethereum consensus, an "epoch" typically means 32 slots, but leanConsensus has no epoch concept at all. The XMSS scheme uses this parameter as a one-time signature index to prevent key reuse, and leanConsensus uses the slot number for this purpose.This is part of the post-devnet-3 alignment work tracked in #155 (pq-devnet-4 preparation).
Description
Pure parameter rename:
epoch→slotacross production code, documentation, and tests. No behavioral or wire-format change — theleansigcrate uses positional arguments, so the external dependency is unaffected.Changes by file
crates/common/types/src/signature.rsis_valid(),sign()crates/common/crypto/src/lib.rsaggregate_signatures(),verify_aggregated_signature()crates/common/crypto/src/lib.rsREAM_EPOCH→REAM_SLOT), test name, comment, and assert message renamescrates/blockchain/src/key_manager.rssign_attestation(),sign_message()crates/blockchain/src/store.rsverify_signature(),verify_aggregated_attestation(), block verificationWhat was NOT renamed
activation_epochvariables in test helpers — these are parameters togenerate_keypair_and_sign()for leansig keypair setup, not part of our signing/verification API.leansiginternals — the external crate's parameter names are unchanged; we only pass our renamed variable positionally.How to test
Related issues
pq-devnet-4#155 (pq-devnet-4 preparation, section 1.2)SignedAttestation.message→.data)3b9d054