Skip to content

Commit 769e1a9

Browse files
tcoratgerclaude
andauthored
docs(lstar): clarify verify_signatures and ban "# Why:" labels (leanEthereum#917)
Tighten the documentation in the lstar block signature verifier: - Rewrite the docstring to spell out the proof contract (the proof holds neither keys nor messages, so the caller rebuilds both) and show the two parallel lists as a layout diagram. - Correct the `Raises:` section: the function raises SpecRejectionError with one of three concrete reasons, not a bare AssertionError. - Add lean, glued one-line comments to each body step (key resolution, message binding, proposer entry, proof check), one sentence per line. - Drop a redundant to_validator_indices() recomputation and rename the loop variables to voter_index/voter_indices to disambiguate from the proposer. Also ban the "# Why:" inline-comment label in the documentation rules: a comment only exists when the reason is non-obvious, so the label is redundant noise. State the reason directly as plain prose. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent fe8cc00 commit 769e1a9

2 files changed

Lines changed: 64 additions & 32 deletions

File tree

.claude/rules/documentation.md

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,13 +168,26 @@ A reader new to the domain should understand the invariant and the data flow wit
168168

169169
Use these labels inside inline comments where they add clarity:
170170

171-
- `# Why:` — when the reason is not obvious from the code.
172171
- `# Invariant:` — the rule being enforced or relied upon.
173172
- `# Threshold:` — for supermajority or quorum arithmetic.
174173
- `# Timing:` — for interval, slot, or epoch calculations.
175174
- `# Phase N:` — for multi-step algorithms.
176175
- `# Fixture state:` — concrete numbers in tests.
177176

177+
NEVER use a `# Why:` label.
178+
A comment exists only when the reason is non-obvious, so labelling it "Why" is redundant noise.
179+
State the reason directly as plain prose.
180+
181+
Bad:
182+
```python
183+
# Why: the bitfield is attacker-controlled, so bound every index first.
184+
```
185+
186+
Good:
187+
```python
188+
# The bitfield is attacker-controlled, so bound every index first.
189+
```
190+
178191
### Concrete values over abstract descriptions
179192

180193
Bad:

src/lean_spec/spec/forks/lstar/signatures.py

Lines changed: 50 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -22,78 +22,97 @@ def verify_signatures(
2222
validators: Validators,
2323
) -> bool:
2424
"""
25-
Verify the merged multi-message aggregate proof carried by a signed block.
25+
Verify the merged aggregate proof carried by a signed block.
2626
27-
The block envelope holds one multi-message aggregate proof binding
28-
every body attestation plus the proposer's signature over the
29-
block root.
27+
A block carries one proof.
28+
It binds every body attestation plus the proposer's endorsement.
29+
30+
The proof itself holds neither public keys nor messages.
31+
It only proves: these keys, signing these messages, produced this aggregate.
32+
So the caller must reconstruct both lists and the verifier checks them.
33+
34+
Two parallel lists drive the check.
35+
Entry i pairs the public keys of one component with the message it signed.
3036
3137
Args:
32-
signed_block: The signed block whose merged proof is checked.
33-
validators: Validator registry providing public keys for verification.
38+
signed_block: Block whose merged proof is verified.
39+
validators: Registry that maps each index to its public keys.
3440
3541
Returns:
36-
True if the merged proof is valid.
42+
True when the proof is valid.
3743
3844
Raises:
39-
AssertionError: On any structural or cryptographic mismatch.
45+
SpecRejectionError: Carrying one of three reasons.
46+
- VALIDATOR_INDEX_OUT_OF_RANGE when an attester index exceeds the registry.
47+
- PROPOSER_INDEX_OUT_OF_RANGE when the proposer index exceeds the registry.
48+
- INVALID_BLOCK_PROOF when the cryptographic check fails.
4049
"""
4150
block = signed_block.block
42-
aggregated_attestations = block.body.attestations
43-
4451
num_validators = Uint64(len(validators))
52+
53+
# Public keys of each signing component, one entry per signed message.
4554
public_keys_per_message: list[list[PublicKey]] = []
4655

47-
# Each component is bound to the message and slot it signed.
56+
# The message each component signed, paired index-for-index with the keys above.
4857
#
49-
# Without this binding a proposer could pair honest signatures
50-
# with attacker-chosen attestation data that resolves to the same
51-
# public_keys, crediting validators for votes they never cast.
58+
# The binding pins each key set to the exact data it signed.
59+
# Without it a proposer could reuse honest signatures over forged data.
5260
message_bindings: list[tuple[Bytes32, Slot]] = []
5361

54-
# One public_key set per attestation, in body order.
55-
#
56-
# The attestation list and the proof component list are parallel.
5762
# Each attestation names the validators that voted for its data.
58-
# Its matching proof component proves those validators signed.
59-
for aggregated_attestation in aggregated_attestations:
60-
validator_indices = aggregated_attestation.aggregation_bits.to_validator_indices()
61-
for validator_index in validator_indices:
62-
if not validator_index.is_within_registry(num_validators):
63+
#
64+
# Invariant: this list stays parallel to the proof's components.
65+
# The producer builds attestations first, in body order, so we match that.
66+
for aggregated_attestation in block.body.attestations:
67+
voter_indices = aggregated_attestation.aggregation_bits.to_validator_indices()
68+
69+
# The bitfield is attacker-controlled so every index is bounds-checked
70+
# against the active set before it indexes the registry below.
71+
for voter_index in voter_indices:
72+
if not voter_index.is_within_registry(num_validators):
6373
raise SpecRejectionError(
6474
RejectionReason.VALIDATOR_INDEX_OUT_OF_RANGE,
6575
"Validator index out of range",
6676
)
6777

78+
# Resolve each voter to the attestation key it signs with.
6879
public_keys_per_message.append(
6980
[
70-
PublicKey.decode_bytes(validators[validator_index].attestation_public_key)
71-
for validator_index in validator_indices
81+
PublicKey.decode_bytes(validators[voter_index].attestation_public_key)
82+
for voter_index in voter_indices
7283
]
7384
)
85+
86+
# Bind that key set to the attestation data root and its slot.
7487
message_bindings.append(
7588
(
7689
hash_tree_root(aggregated_attestation.data),
7790
aggregated_attestation.data.slot,
7891
)
7992
)
8093

81-
# Final component: the proposer's signature over the block root.
94+
# The proposer's endorsement is the final component, appended last.
8295
#
83-
# The proposer signs the block root with their proposal key.
84-
# This proves the proposer endorsed this specific block.
85-
# It is a single-participant entry, distinct from the vote entries.
86-
proposer_index = block.proposer_index
87-
if not proposer_index.is_within_registry(num_validators):
96+
# It is the only thing tying the proof to this specific block.
97+
# It signs the block root with the proposal key, not the attestation key.
98+
# So it stands as a single-participant entry distinct from the votes above.
99+
#
100+
# The proposer index is attacker-controlled, so bound it before indexing.
101+
if not block.proposer_index.is_within_registry(num_validators):
88102
raise SpecRejectionError(
89103
RejectionReason.PROPOSER_INDEX_OUT_OF_RANGE, "Proposer index out of range"
90104
)
91105

106+
# Resolve the proposal key, the lone signer of this component.
92107
public_keys_per_message.append(
93-
[PublicKey.decode_bytes(validators[proposer_index].proposal_public_key)]
108+
[PublicKey.decode_bytes(validators[block.proposer_index].proposal_public_key)]
94109
)
110+
111+
# Bind it to the block root and the block's own slot.
95112
message_bindings.append((hash_tree_root(block), block.slot))
96113

114+
# Check the proof against the reconstructed keys and messages.
115+
# A failure here means the aggregate does not match what the block claims.
97116
try:
98117
signed_block.proof.verify(
99118
public_keys_per_message=public_keys_per_message,

0 commit comments

Comments
 (0)