Skip to content

test: verify real signatures in fork-choice spec tests #304

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.

Sign up for GitHub

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
MegaRedHand wants to merge 1 commit into
base: pr-fixture-schema
Choose a base branch
from
+129 −158
Open

test: verify real signatures in fork-choice spec tests #304

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 13 additions & 40 deletions crates/blockchain/src/store.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -602,16 +602,16 @@ pub fn on_gossip_attestation(
/// Process a gossiped aggregated attestation from the aggregation subnet.
///
/// Aggregated attestations arrive from committee aggregators and contain a proof
/// covering multiple validators. We store one aggregated payload entry per
/// participating validator so the fork choice extraction works uniformly.
/// covering multiple validators. After signature verification, one entry is
/// stored per unique attestation data (not per participating validator) in the
/// pending pool; participant bits are carried in the proof itself.
pub fn on_gossip_aggregated_attestation(
store: &mut Store,
aggregated: SignedAggregatedAttestation,
) -> Result<(), StoreError> {
validate_attestation_data(store, &aggregated.data)
.inspect_err(|_| metrics::inc_attestations_invalid())?;

// Verify aggregated proof signature
let target_state = store
.get_state(&aggregated.data.target.root)
.ok_or(StoreError::MissingTargetState(aggregated.data.target.root))?;
Expand Down Expand Up @@ -647,18 +647,22 @@ pub fn on_gossip_aggregated_attestation(
}
.map_err(StoreError::AggregateVerificationFailed)?;

// Store one proof per attestation data (not per validator)
store.insert_new_aggregated_payload(hashed, aggregated.proof.clone());
// Read stats before moving the proof into the store.
let num_participants = aggregated.proof.participants.count_ones();
let target_slot = aggregated.data.target.slot;
let target_root = aggregated.data.target.root;
let source_slot = aggregated.data.source.slot;
let slot = aggregated.data.slot;

store.insert_new_aggregated_payload(hashed, aggregated.proof);
metrics::update_latest_new_aggregated_payloads(store.new_aggregated_payloads_count());

let slot = aggregated.data.slot;
info!(
slot,
num_participants,
target_slot = aggregated.data.target.slot,
target_root = %ShortRoot(&aggregated.data.target.root.0),
source_slot = aggregated.data.source.slot,
target_slot,
target_root = %ShortRoot(&target_root.0),
source_slot,
"Aggregated attestation processed"
);

Expand Down Expand Up @@ -686,37 +690,6 @@ pub fn on_block_without_verification(
on_block_core(store, signed_block, false)
}

/// Process a gossip attestation without signature verification.
///
/// Validates the attestation data and inserts it directly into the known
/// attestation payloads (bypassing the gossip → aggregate → promote pipeline).
/// Use only in tests where signatures are absent (e.g., fork choice spec tests).
pub fn on_gossip_attestation_without_verification(
store: &mut Store,
validator_id: u64,
data: AttestationData,
) -> Result<(), StoreError> {
validate_attestation_data(store, &data)?;

// Validate the validator index exists in the target state
let target_state = store
.get_state(&data.target.root)
.ok_or(StoreError::MissingTargetState(data.target.root))?;
if validator_id >= target_state.validators.len() as u64 {
return Err(StoreError::InvalidValidatorIndex);
}

let bits = aggregation_bits_from_validator_indices(&[validator_id]);
let proof = AggregatedSignatureProof::empty(bits);
let hashed = HashedAttestationData::new(data);
store.insert_known_aggregated_payload(hashed, proof);

// Recalculate fork choice head after inserting the attestation
update_head(store, false);

Ok(())
}

/// Core block processing logic.
///
/// When `verify` is true, cryptographic signatures are validated and stored
Expand Down
152 changes: 60 additions & 92 deletions crates/blockchain/tests/forkchoice_spectests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ use std::{
use ethlambda_blockchain::{MILLISECONDS_PER_INTERVAL, MILLISECONDS_PER_SLOT, store};
use ethlambda_storage::{Store, backend::InMemoryBackend};
use ethlambda_types::{
attestation::{AttestationData, XmssSignature},
attestation::{AttestationData, SignedAggregatedAttestation, SignedAttestation, XmssSignature},
block::{AggregatedSignatureProof, Block, BlockSignatures, SignedBlock},
primitives::{H256, HashTreeRoot as _},
primitives::{ByteList, H256, HashTreeRoot as _},
signature::SIGNATURE_SIZE,
state::State,
};
Expand All @@ -21,29 +21,32 @@ const SUPPORTED_FIXTURE_FORMAT: &str = "fork_choice_test";
mod common;
mod types;

// We don't check signatures in spec-tests, so invalid signature tests always pass.
// The gossipAggregatedAttestation/attestation tests fail because the harness inserts
// individual gossip attestations into known payloads (should be no-op) and aggregated
// attestations with validator_id=0 into known (should use proof.participants into new).
// The last three skips are fixtures whose attestation checks require the harness to
// route `gossipAggregatedAttestation` steps through the real aggregated path (see the
// follow-up PR). They're unblocked there.
// TODO: fix these
// Tests skipped until the leanMultisig fork mismatch is resolved: proofs in
// these fixtures are generated by `anshalshukla/leanMultisig@devnet-4`, while
// our verifier is pinned to `leanEthereum/leanMultisig@2eb4b9d`. The two
// disagree on the Fiat-Shamir `public_input` encoding and the aggregation
// bytecode program, so every proof rejects with `InvalidProof`.
const SKIP_TESTS: &[&str] = &[
"test_gossip_attestation_with_invalid_signature",
"test_block_builder_fixed_point_advances_justification",
"test_equivocating_proposer_with_split_attestations",
"test_finalization_prunes_stale_aggregated_payloads",
"test_finalization_prunes_stale_attestation_signatures",
"test_produce_block_enforces_max_attestations_data_limit",
"test_produce_block_includes_pending_attestations",
"test_safe_target_advances_incrementally_along_the_chain",
"test_safe_target_does_not_advance_below_supermajority",
"test_safe_target_follows_heavier_fork_on_split",
"test_safe_target_is_conservative_relative_to_lmd_ghost_head",
"test_safe_target_uses_merged_pools_at_interval_3",
"test_tick_interval_0_skips_acceptance_when_not_proposer",
"test_tick_interval_progression_through_full_slot",
"test_valid_gossip_aggregated_attestation",
];

fn run(path: &Path) -> datatest_stable::Result<()> {
if let Some(stem) = path.file_stem().and_then(|s| s.to_str())
&& SKIP_TESTS.contains(&stem)
{
println!("Skipping {stem} (gossip attestation not serialized in fixture)");
println!("Skipping {stem} (see SKIP_TESTS comment)");
return Ok(());
}
let tests = ForkChoiceTestVector::from_file(path)?;
Expand Down Expand Up @@ -89,25 +92,7 @@ fn run(path: &Path) -> datatest_stable::Result<()> {
// NOTE: the has_proposal argument is set to true, following the spec
store::on_tick(&mut store, block_time_ms, true, false);
let result = store::on_block_without_verification(&mut store, signed_block);

match (result.is_ok(), step.valid) {
(true, false) => {
return Err(format!(
"Step {} expected failure but got success",
step_idx
)
.into());
}
(false, true) => {
return Err(format!(
"Step {} expected success but got failure: {:?}",
step_idx,
result.err()
)
.into());
}
_ => {}
}
assert_step_outcome(step_idx, step.valid, result)?;
}
"tick" => {
// Fixtures use either `time` (UNIX seconds) or `interval`
Expand All @@ -127,75 +112,44 @@ fn run(path: &Path) -> datatest_stable::Result<()> {
let att_data = step
.attestation
.expect("attestation step missing attestation data");
let domain_data: ethlambda_types::attestation::AttestationData =
att_data.data.into();
let validator_id = att_data
.validator_id
.expect("attestation step missing validator_id");
let signed_attestation = SignedAttestation {
validator_id: att_data
.validator_id
.expect("attestation step missing validator_id"),
data: att_data.data.into(),
signature: att_data
.signature
.expect("attestation step missing signature"),
};
Comment thread
MegaRedHand marked this conversation as resolved.
let is_aggregator = step.is_aggregator.unwrap_or(false);

let result = store::on_gossip_attestation_without_verification(
let result = store::on_gossip_attestation(
&mut store,
validator_id,
domain_data,
&signed_attestation,
is_aggregator,
);

match (result.is_ok(), step.valid) {
(true, false) => {
return Err(format!(
"Step {} expected failure but got success",
step_idx
)
.into());
}
(false, true) => {
return Err(format!(
"Step {} expected success but got failure: {:?}",
step_idx,
result.err()
)
.into());
}
_ => {}
}
assert_step_outcome(step_idx, step.valid, result)?;
}
"gossipAggregatedAttestation" => {
// Aggregated attestation fixtures now carry proof data with a
// participants bitfield, but the harness still uses the
// single-validator bypass here. Tests whose checks rely on the
// correct participants or pool routing are skipped via
// `SKIP_TESTS`; the follow-up PR wires the real verifying
// path through.
let att_data = step
.attestation
.expect("gossipAggregatedAttestation step missing attestation data");
let domain_data: ethlambda_types::attestation::AttestationData =
att_data.data.into();
let validator_id = att_data.validator_id.unwrap_or(0);

let result = store::on_gossip_attestation_without_verification(
&mut store,
validator_id,
domain_data,
);
let proof_fixture = att_data
.proof
.expect("gossipAggregatedAttestation step missing proof");
let proof_bytes: Vec<u8> = proof_fixture.proof_data.into();
let proof_data = ByteList::try_from(proof_bytes)
.expect("aggregated proof data fits in ByteListMiB");
let aggregated = SignedAggregatedAttestation {
data: att_data.data.into(),
proof: AggregatedSignatureProof::new(
proof_fixture.participants.into(),
proof_data,
),
};

match (result.is_ok(), step.valid) {
(true, false) => {
return Err(format!(
"Step {} expected failure but got success",
step_idx
)
.into());
}
(false, true) => {
return Err(format!(
"Step {} expected success but got failure: {:?}",
step_idx,
result.err()
)
.into());
}
_ => {}
}
let result = store::on_gossip_aggregated_attestation(&mut store, aggregated);
assert_step_outcome(step_idx, step.valid, result)?;
}
other => {
return Err(format!("Unsupported step type '{other}'").into());
Expand All @@ -211,6 +165,20 @@ fn run(path: &Path) -> datatest_stable::Result<()> {
Ok(())
}

fn assert_step_outcome<T, E: std::fmt::Debug>(
step_idx: usize,
expected_valid: bool,
result: Result<T, E>,
) -> datatest_stable::Result<()> {
match (result, expected_valid) {
(Ok(_), false) => Err(format!("Step {step_idx} expected failure but got success").into()),
(Err(err), true) => {
Err(format!("Step {step_idx} expected success but got failure: {err:?}").into())
}
_ => Ok(()),
}
}

fn build_signed_block(block_data: types::BlockStepData) -> SignedBlock {
let block: Block = block_data.to_block();

Expand Down
23 changes: 1 addition & 22 deletions crates/blockchain/tests/signature_types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::common::{AggregationBits, Block, Container, TestInfo, TestState};
use super::common::{AggregationBits, Block, Container, TestInfo, TestState, deser_xmss_hex};
use ethlambda_types::attestation::{AggregationBits as EthAggregationBits, XmssSignature};
use ethlambda_types::block::{
AggregatedSignatureProof, AttestationSignatures, BlockSignatures, SignedBlock,
Expand Down Expand Up @@ -110,24 +110,3 @@ pub struct AttestationSignature {
pub struct ProofData {
pub data: String,
}

// ============================================================================
// Helpers
// ============================================================================

pub fn deser_xmss_hex<'de, D>(d: D) -> Result<XmssSignature, D::Error>
where
D: serde::Deserializer<'de>,
{
use serde::de::Error;

let value = String::deserialize(d)?;
let bytes = hex::decode(value.strip_prefix("0x").unwrap_or(&value))
.map_err(|_| D::Error::custom("XmssSignature value is not valid hex"))?;
XmssSignature::try_from(bytes).map_err(|_| {
D::Error::custom(format!(
"XmssSignature length != {}",
ethlambda_types::signature::SIGNATURE_SIZE
))
})
}
43 changes: 39 additions & 4 deletions crates/blockchain/tests/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use super::common::{self, Block, TestInfo, TestState};
use super::common::{self, Block, TestInfo, TestState, deser_xmss_hex};
use ethlambda_types::attestation::XmssSignature;
use ethlambda_types::primitives::H256;
use serde::Deserialize;
use serde::{Deserialize, Deserializer};
use std::collections::HashMap;
use std::path::Path;

Expand Down Expand Up @@ -59,15 +60,49 @@ pub struct ForkChoiceStep {
pub interval: Option<u64>,
#[serde(rename = "hasProposal")]
pub has_proposal: Option<bool>,
#[serde(rename = "isAggregator")]
pub is_aggregator: Option<bool>,
}

#[derive(Debug, Clone, Deserialize)]
pub struct AttestationStepData {
#[serde(rename = "validatorId")]
pub validator_id: Option<u64>,
pub data: common::AttestationData,
#[allow(dead_code)]
pub signature: Option<String>,
#[serde(default, deserialize_with = "deser_opt_xmss_hex")]
pub signature: Option<XmssSignature>,
/// Present on `gossipAggregatedAttestation` steps.
pub proof: Option<ProofStepData>,
}

#[derive(Debug, Clone, Deserialize)]
pub struct ProofStepData {
pub participants: common::AggregationBits,
#[serde(rename = "proofData")]
pub proof_data: HexByteList,
}

/// Hex-encoded byte list in the fixture format: `{ "data": "0xdeadbeef" }`.
#[derive(Debug, Clone, Deserialize)]
pub struct HexByteList {
data: String,
}

impl From<HexByteList> for Vec<u8> {
fn from(value: HexByteList) -> Self {
let stripped = value.data.strip_prefix("0x").unwrap_or(&value.data);
hex::decode(stripped).expect("invalid hex in proof data")
}
}

fn deser_opt_xmss_hex<'de, D>(d: D) -> Result<Option<XmssSignature>, D::Error>
where
D: Deserializer<'de>,
{
#[derive(Deserialize)]
struct Wrap(#[serde(deserialize_with = "deser_xmss_hex")] XmssSignature);

Ok(Option::<Wrap>::deserialize(d)?.map(|w| w.0))
}

#[derive(Debug, Clone, Deserialize)]
Expand Down
Loading
Loading