Skip to content

test: add ignored VRF output uniqueness regression test (QUI-925) - #6

Open
rcarback wants to merge 1 commit into
v0.2from
feature/qui-925
Open

rcarback wants to merge 1 commit into
v0.2from
feature/qui-925

Conversation

@rcarback

@rcarback rcarback commented Aug 7, 2026

Copy link
Copy Markdown

Adds one #[ignore]d test that pins the VRF uniqueness property BABE depends on, plus the rand_core dev-dependency it needs.

What the test does

hybrid_vrf_output_is_independent_of_the_pq_binding signs a BABE VRF input through the normal path, then re-signs the same binding message with signing randomness of its own choosing. It asserts three things in order:

  1. The two ML-DSA-44 bindings differ.
  2. vrf_verify accepts the second one.
  3. The bytes from make_bytes match for both.

Assertion 3 fails today. Assertions 1 and 2 pass, which is the point. The verifier accepts a binding the signer chose, and the consensus-decisive bytes move with it.

Why it is ignored

The test fails against the current derivation. That failure is the defect, not a broken test. VrfOutput::from_parts computes SHA256(sr25519_pre_output || ml_dsa_signature), and ML-DSA-44 is not unique per (key, message), so a block producer can re-sign until the authoring score clears the primary threshold.

Marking it #[ignore] keeps the suite green while the property stays recorded in code. Default runs print the reason:

test ... hybrid_vrf_output_is_independent_of_the_pq_binding ... ignored,
  QUI-925: expected to fail until QUI-984 removes the PQ binding from the
  consensus-decisive derivation

QUI-984 makes it pass by deriving make_bytes from the unique sr25519 pre-output alone. Remove #[ignore] as part of that change. The test must then pass unmodified.

Why the existing tests do not cover this

hybrid_vrf_roundtrip_works and hybrid_vrf_output_matches_signed_proof_output exercise only the honest signer, which calls sign_deterministic. They pass on the grindable construction. hybrid_vrf_rejects_tampered_pq_binding checks that a corrupted binding is rejected, which is a different property from uniqueness.

Verification

test result: ok. 12 passed; 0 failed; 1 ignored

Run with --ignored, the test fails on assertion 3. The RNG is a deterministic counter, so the failure output is byte-stable across runs.

Notes

rand_core moved to quip-crypto-primitives-core in 64270a9, so this adds it back as a dev-dependency of quip-crypto-primitives, pinned at 0.6 to match what fips204 expects.

cargo fmt --check reports diffs across the whole quip/ tree, including files this PR does not touch. The tree uses four spaces while the repo rustfmt config selects tabs. That is pre-existing and left alone.

Tracked by QUI-925. Blocked by QUI-984.

BABE derives its authoring score and epoch-randomness contribution from
SHA256(sr25519_pre_output || ml_dsa_signature). ML-DSA-44 is not unique
per (key, message), so a block producer can re-sign the same binding
message with randomness of their choosing until the score clears the
primary threshold.

The existing tests cannot catch this. They exercise only the honest
signer, which calls sign_deterministic, so they pass on the grindable
construction too.

This test mints a second valid binding with signer-chosen randomness,
asserts the verifier accepts it, then asserts the derived consensus
bytes match. It is marked #[ignore] because it fails against the
current derivation, which is the defect itself rather than a broken
test. QUI-984 makes it pass by deriving make_bytes from the unique
sr25519 pre-output alone. Remove #[ignore] as part of that change.
@linear

linear Bot commented Aug 7, 2026

Copy link
Copy Markdown

QUI-925

@augmentcode

augmentcode Bot commented Aug 7, 2026

Copy link
Copy Markdown
🤖 Augment PR Summary

Summary: This PR records the BABE hybrid-VRF output uniqueness regression described in QUI-925.

Changes:

  • Adds an ignored test that re-signs one valid SR25519 VRF pre-output with chosen ML-DSA-44 randomness.
  • Verifies the alternate binding is distinct and valid, then asserts both proofs derive identical consensus bytes.
  • Adds a direct rand_core 0.6 dev-dependency for the deterministic test RNG.

Technical Notes: The test is intentionally ignored because the current derivation includes the non-unique PQ signature; QUI-984 is expected to make it pass unchanged.

🤖 Was this summary useful? React with 👍 or 👎

@augmentcode augmentcode Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review completed. No suggestions at this time.

Comment augment review to trigger a new review at any time.

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