Skip to content

refactor(forks/lstar): move multi-signature types out of crypto layer#796

Merged
tcoratger merged 2 commits into
leanEthereum:mainfrom
tcoratger:refactor/multisig-types-out-of-crypto
May 29, 2026
Merged

refactor(forks/lstar): move multi-signature types out of crypto layer#796
tcoratger merged 2 commits into
leanEthereum:mainfrom
tcoratger:refactor/multisig-types-out-of-crypto

Conversation

@tcoratger
Copy link
Copy Markdown
Collaborator

Summary

The crypto subspec imported AggregationBits, Slot, ValidatorIndex, and ValidatorIndices from the consensus layer to define TypeOneMultiSignature and TypeTwoMultiSignature. That layering inversion forced three mid-file deferred imports with # noqa: E402 in lstar/containers.py and let the crypto layer reach into consensus for types it should never know about.

This PR moves the multi-signature classes (plus AggregationError) into the consensus layer, where they belong as domain-typed wrappers around the crypto byte-level primitives.

What moved where

Class Before After
AggregationError crypto/xmss/aggregation.py lstar/containers.py
TypeOneMultiSignature crypto/xmss/aggregation.py lstar/containers.py
TypeTwoMultiSignature crypto/xmss/aggregation.py lstar/containers.py
Slot, IMMEDIATE_JUSTIFICATION_WINDOW lstar/containers.py new lstar/slot.py (re-exported from containers for callers)

crypto/xmss/aggregation.py shrinks from 348 lines to 38 — only Rust prover bindings, LOG_INV_RATE, and the one-time setup_prover call remain. No consensus imports.

Slot moves into its own small module so the crypto layer can still bind signatures to a Slot (not a Uint64) without pulling the full consensus container module. Semantics preserved across the public crypto API.

Cycle resolution

Before:

# lstar/containers.py:178-180
from lean_spec.node.chain.config import HISTORICAL_ROOTS_LIMIT  # noqa: E402
from lean_spec.spec.crypto.xmss.aggregation import TypeOneMultiSignature  # noqa: E402
from lean_spec.spec.crypto.xmss.containers import PublicKey, Signature  # noqa: E402

After: all three previously deferred imports sit at the top of lstar/containers.py. The # noqa: E402 block is gone.

Importer updates

17 files updated to fetch TypeOneMultiSignature, TypeTwoMultiSignature, and AggregationError from the consensus layer instead of the crypto layer. Two crypto-side files (crypto/xmss/{containers,interface}.py) now import Slot from lstar/slot.py.

Test plan

  • just check — ruff, format, ty, codespell, mdformat pass
  • uv run pytest tests/lean_spec/spec/crypto/xmss/ tests/lean_spec/spec/forks/lstar/ tests/lean_spec/node/{validator,sync}/ tests/lean_spec/helpers/ — 628 tests pass
  • CI green on this PR

Net change: +455/−437 across 25 files (new slot.py plus the migrations and importer updates).

🤖 Generated with Claude Code

tcoratger and others added 2 commits May 29, 2026 14:45
The crypto subspec imported AggregationBits, Slot, ValidatorIndex, and
ValidatorIndices from the consensus layer to define TypeOneMultiSignature
and TypeTwoMultiSignature. That layering inversion forced three mid-file
deferred imports with # noqa: E402 in lstar/containers.py and made the
crypto layer reach into consensus for types it should never know about.

Move the multi-signature classes (plus AggregationError) into the
consensus layer, where they belong as domain-typed wrappers around the
crypto byte-level primitives. The crypto layer keeps only the Rust
prover bindings.

Slot moves into its own small module so the crypto layer can name a
slot without pulling the full consensus container module. The crypto
API still uses Slot, not Uint64 — the layering fix preserves semantics.

Net result:
- crypto/xmss/aggregation.py shrinks from 348 lines to 38; no consensus
  imports remain.
- crypto/xmss/{containers,interface}.py import Slot from lstar/slot.py.
- lstar/containers.py promotes all three previously deferred imports
  (HISTORICAL_ROOTS_LIMIT, multi-sig types, PublicKey/Signature) to the
  top of the file. The # noqa: E402 block is gone.
- 17 importers updated to fetch TypeOneMultiSignature etc. from the
  consensus layer.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After the multi-signature containers moved to the consensus layer, the
aggregation module shrank to a re-export shell with one import-time
side effect. With exactly one consumer (lstar/containers.py), the
indirection added cost without abstraction.

Distribute the three concerns:

- Rust binding imports go directly into lstar/containers.py from
  lean_multisig_py.
- LOG_INV_RATE moves next to its sole caller in lstar/containers.py.
- setup_prover(mode=LEAN_ENV) moves into crypto/xmss/__init__.py,
  alongside the other LEAN_ENV-driven xmss bootstrap.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@tcoratger tcoratger merged commit 374c6aa into leanEthereum:main May 29, 2026
13 checks passed
tcoratger added a commit to unnawut/leanSpec that referenced this pull request May 30, 2026
…renames

Rebase onto main and apply the renames/cleanups landed since the branch
was opened:

- PR leanEthereum#799: TypeOneMultiSignature → SingleMessageAggregate, proof_type
  literal "type_1" → "single_message", file renames
  test_type_1_{valid,invalid}.py → test_single_message_{valid,invalid}.py.
- PR leanEthereum#800: validator_ids → validator_indices, with_validator_id →
  with_validator_index, vid/pubkey expansions.
- post-leanEthereum#788/leanEthereum#790/leanEthereum#796 imports: lean_spec.spec.forks / .spec.ssz /
  .spec.crypto.*.

Replace the stringly-typed tamper dict with a Pydantic discriminated
union (RebindToAlternateHeadRoot, IncrementEmittedSlot,
SwapParticipantPublicKey). The match dispatch on the typed variants
drops the two type: ignore[index] casts and lets the test sites read
tamper=SwapParticipantPublicKey(index=0, with_validator_index=1)
instead of a magic-string dict.

Inline the four single-call helpers (_apply_tamper plus three
_tamper_*) and the verification helper into make_fixture so the four
phases — generate / tamper / verify / publish — are visible in one
method.

Drop both model_copy(update=...) calls per leanEthereum#789: direct field
construction for the frozen AttestationData rebuild, direct field
assignment for the mutable fixture self-update.

Replace the internal dict[str, Any] bundle with named locals; the
bundle never escapes make_fixture, so the dict adds nothing.

Guard SwapParticipantPublicKey against silent no-op swaps where the
replacement key happens to equal the original. Broaden the verifier
exception catch to surface unexpected exception types as
"expected X got Y" instead of crashing the filler.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
tcoratger added a commit that referenced this pull request May 30, 2026
…heck proof verification (#786)

* test(consensus): add VerifyProofsTest fixture and Type-1 valid vectors

Introduces a new consensus fixture format that emits self-contained
multi-signature verification vectors so cross-client implementations
can run their own Type-1 verifier and compare outcomes.

Three positive vectors land alongside the fixture: a single-validator
baseline, a four-validator all-participating case, and a
four-validator non-contiguous bitfield case ([1, 0, 1, 1]).

The fixture also surfaces the spec-layer binding between attestation
data and proof: clients recompute hash_tree_root(attestation_data)
and must match the emitted message field before running the verifier.

* test(consensus): add Type-1 verify_proofs rejection vectors

Adds four negative vectors exercising spec-layer bindings between
inputs and the multi-signature proof. Each vector uses a tamper
operation on the fixture to produce a structurally valid bundle that
must be rejected by a conformant verifier:

- wrong_message: proof bound to an alternate head root inside the
  attestation data
- wrong_slot: emitted slot field shifted while the proof binding
  stays on the original slot
- wrong_public_keys: one emitted pubkey replaced with another
  validator's
- aggregation_bits_length_mismatch: emitted bits truncated while
  the pubkey count stays unchanged

Vectors covering malformed or truncated proof bytes are intentionally
out of scope: leanSpec consumes the multi-signature primitive as a
black box and primitive integrity belongs to its own conformance
suite. Pubkey ordering is also not a binding to test: the aggregator
sorts participants internally, so the verifier is order-insensitive.

* test(consensus): align VerifyProofsTest with sibling fixture conventions

Brings the new fixture in line with the patterns the other consensus
test fixtures follow:

- Drop ``from __future__ import annotations`` (PR #759 removed it from
  Pydantic-defining files); quote the one self-reference instead.
- Replace the bespoke ``expect_valid: bool`` field with the inherited
  ``expect_exception`` field already used by SSZTest, NetworkingCodec,
  and VerifySignaturesTest. Rejection vectors now pin
  ``AggregationError`` and the framework serializes the class name to
  JSON.
- Switch the tamper dispatch in ``_apply_tamper`` from ``if/elif`` to
  ``match/case`` to follow the pattern in slot_clock and
  networking_codec.
- Expand the module-level docstring from one line to a short
  paragraph describing what the fixture covers.
- Normalize the ``public_keys`` default from ``[]`` to ``| None =
  None`` to match every other output field on the model.

* test(consensus): drop aggregation_bits_length_mismatch rejection vector

The check that fires here is the early-reject in the spec wrapper's
verify method (len(public_keys) != participants.count(True)), not a
consensus-critical binding. In real consensus the inconsistency cannot
arise because clients resolve public keys from the bitfield plus the
validator registry as one operation. A client that did pass a wrong
pubkey count would also be rejected by the underlying recursive
verifier on its internal pubkey-set commitment, so the wrapper check
is at best an early exit with a nicer error message.

The remaining three rejection vectors still exercise the meaningful
spec-layer bindings: message hash, slot, and pubkey set.

* refactor(consensus): tighten VerifyProofsTest and absorb post-rebase renames

Rebase onto main and apply the renames/cleanups landed since the branch
was opened:

- PR #799: TypeOneMultiSignature → SingleMessageAggregate, proof_type
  literal "type_1" → "single_message", file renames
  test_type_1_{valid,invalid}.py → test_single_message_{valid,invalid}.py.
- PR #800: validator_ids → validator_indices, with_validator_id →
  with_validator_index, vid/pubkey expansions.
- post-#788/#790/#796 imports: lean_spec.spec.forks / .spec.ssz /
  .spec.crypto.*.

Replace the stringly-typed tamper dict with a Pydantic discriminated
union (RebindToAlternateHeadRoot, IncrementEmittedSlot,
SwapParticipantPublicKey). The match dispatch on the typed variants
drops the two type: ignore[index] casts and lets the test sites read
tamper=SwapParticipantPublicKey(index=0, with_validator_index=1)
instead of a magic-string dict.

Inline the four single-call helpers (_apply_tamper plus three
_tamper_*) and the verification helper into make_fixture so the four
phases — generate / tamper / verify / publish — are visible in one
method.

Drop both model_copy(update=...) calls per #789: direct field
construction for the frozen AttestationData rebuild, direct field
assignment for the mutable fixture self-update.

Replace the internal dict[str, Any] bundle with named locals; the
bundle never escapes make_fixture, so the dict adds nothing.

Guard SwapParticipantPublicKey against silent no-op swaps where the
replacement key happens to equal the original. Broaden the verifier
exception catch to surface unexpected exception types as
"expected X got Y" instead of crashing the filler.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: Thomas Coratger <60488569+tcoratger@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
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.

1 participant