From a3c0954cff71e57072ba43590ffa61fd820d3708 Mon Sep 17 00:00:00 2001 From: petarjuki7 Date: Wed, 8 Jul 2026 13:56:55 +0200 Subject: [PATCH 1/9] feat(validator_store): implement sign_proposer_preferences MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sign ProposerPreferences via a single-validator partial-signature collection round: domain keyed on proposal_slot's epoch, envelope slot stamped with the duty's proposal_slot (SIP-94 §5/§7, not the send slot), no slashing-DB interaction. Add a reconstruction-failure reporter and metric, and generalize the shared collect_signature failure classifier. Part of #1063. --- anchor/validator_store/src/instrumentation.rs | 16 +- anchor/validator_store/src/lib.rs | 112 +++++- anchor/validator_store/src/metrics.rs | 25 ++ anchor/validator_store/src/testing/mod.rs | 1 + .../src/testing/proposer_preferences.rs | 371 ++++++++++++++++++ 5 files changed, 510 insertions(+), 15 deletions(-) create mode 100644 anchor/validator_store/src/testing/proposer_preferences.rs diff --git a/anchor/validator_store/src/instrumentation.rs b/anchor/validator_store/src/instrumentation.rs index 2e3338f36..5882ea5ff 100644 --- a/anchor/validator_store/src/instrumentation.rs +++ b/anchor/validator_store/src/instrumentation.rs @@ -23,12 +23,16 @@ pub mod checkpoints { pub const DUTY_FAILED: &str = "duty_failed"; } -/// Telemetry classification of `collect_signature` failures during PTC duties. +/// Telemetry classification of `collect_signature` failures. +/// +/// Shared by every single-validator signing path (PTC, ProposerPreferences, ...): the +/// classification depends only on the generic `collect_signature` error, never on the role, so +/// each caller maps these classes onto its own log message and reconstruction-failure metric. /// /// Returned as an enum rather than a label string because the class drives two independent /// effects in the caller: the log level and whether a reconstruction-failure metric is /// incremented at all. -pub enum PtcFailureClass { +pub enum CollectionFailureClass { /// The committee never reached the partial signature threshold. This surfaces as /// `QueueClosedError` because the collector is evicted after /// `SIGNATURE_COLLECTOR_RETAIN_SLOTS`, dropping the result channel while we await it, so it @@ -41,22 +45,22 @@ pub enum PtcFailureClass { NonCollection, } -pub fn classify_ptc_collection_failure(error: &Error) -> PtcFailureClass { +pub fn classify_collection_failure(error: &Error) -> CollectionFailureClass { match error { // The inner match is deliberately wildcard-free so a future `CollectionError` variant // forces a conscious classification decision here at compile time. Error::SpecificError(SpecificError::SignatureCollectionFailed(collection_error)) => { match collection_error { CollectionError::QueueClosedError | CollectionError::CollectionTimeout => { - PtcFailureClass::NoSignature + CollectionFailureClass::NoSignature } CollectionError::QueueFullError | CollectionError::OwnOperatorIdUnknown | CollectionError::EmptySignature - | CollectionError::RecoverError(_) => PtcFailureClass::Infra, + | CollectionError::RecoverError(_) => CollectionFailureClass::Infra, } } - _ => PtcFailureClass::NonCollection, + _ => CollectionFailureClass::NonCollection, } } diff --git a/anchor/validator_store/src/lib.rs b/anchor/validator_store/src/lib.rs index c3a158ac7..ab07f26c6 100644 --- a/anchor/validator_store/src/lib.rs +++ b/anchor/validator_store/src/lib.rs @@ -81,7 +81,7 @@ use validator_store::{ ValidatorStore, }; -use crate::instrumentation::PtcFailureClass; +use crate::instrumentation::CollectionFailureClass; /// Number of epochs of slashing protection history to keep. /// @@ -98,6 +98,7 @@ const AGGREGATE_LOG_NAME: &str = "aggregate"; const SELECTION_PROOF_LOG_NAME: &str = "selection proof"; const SYNC_SELECTION_PROOF_LOG_NAME: &str = "sync selection proof"; const SYNC_COMMITTEE_CONTRIBUTION_LOG_NAME: &str = "sync committee contribution"; +const PROPOSER_PREFERENCES_LOG_NAME: &str = "proposer preferences"; /// A request to collect a committee signature for a single validator. /// @@ -871,8 +872,8 @@ impl + 'static> AnchorValidator validator_pubkey: &PublicKeyBytes, slot: Slot, ) { - match instrumentation::classify_ptc_collection_failure(error) { - PtcFailureClass::NoSignature => { + match instrumentation::classify_collection_failure(error) { + CollectionFailureClass::NoSignature => { warn!( ?validator_pubkey, %slot, @@ -884,7 +885,7 @@ impl + 'static> AnchorValidator &[metrics::PTC_FAILURE_NO_SIGNATURE], ); } - PtcFailureClass::Infra => { + CollectionFailureClass::Infra => { error!( ?validator_pubkey, %slot, @@ -896,7 +897,7 @@ impl + 'static> AnchorValidator &[metrics::PTC_FAILURE_INFRA], ); } - PtcFailureClass::NonCollection => { + CollectionFailureClass::NonCollection => { error!( ?validator_pubkey, %slot, @@ -907,6 +908,53 @@ impl + 'static> AnchorValidator } } + /// Classify and report a ProposerPreferences signature-collection failure. + fn report_proposer_preferences_collection_failure( + &self, + error: &Error, + preferences: &ProposerPreferences, + signing_root: Hash256, + ) { + match instrumentation::classify_collection_failure(error) { + CollectionFailureClass::NoSignature => { + warn!( + validator_index = preferences.validator_index, + proposal_slot = %preferences.proposal_slot, + target_gas_limit = preferences.target_gas_limit, + dependent_root = ?preferences.dependent_root, + ?signing_root, + ?error, + "ProposerPreferences reconstruction failed; operators likely diverged on the \ + signing root (target_gas_limit or dependent_root)" + ); + metrics::inc_counter_vec( + &metrics::PROPOSER_PREFERENCES_RECONSTRUCTION_FAILURES, + &[metrics::PROPOSER_PREFERENCES_FAILURE_SIGNING_ROOT_DIVERGENCE], + ); + } + CollectionFailureClass::Infra => { + error!( + validator_index = preferences.validator_index, + proposal_slot = %preferences.proposal_slot, + ?error, + "ProposerPreferences signature collection infrastructure failure" + ); + metrics::inc_counter_vec( + &metrics::PROPOSER_PREFERENCES_RECONSTRUCTION_FAILURES, + &[metrics::PROPOSER_PREFERENCES_FAILURE_INFRA], + ); + } + CollectionFailureClass::NonCollection => { + error!( + validator_index = preferences.validator_index, + proposal_slot = %preferences.proposal_slot, + ?error, + "Failed to sign ProposerPreferences" + ); + } + } + } + fn create_proposer_consensus_data_validator( &self, validator_pubkey: PublicKeyBytes, @@ -3487,11 +3535,57 @@ impl + 'static> ValidatorStore async fn sign_proposer_preferences( &self, - _validator_pubkey: PublicKeyBytes, - _preferences: ProposerPreferences, + validator_pubkey: PublicKeyBytes, + preferences: ProposerPreferences, ) -> Result { - // TODO(gloas) - Err(Error::SpecificError(SpecificError::Unsupported)) + let (validator, cluster) = self.get_validator_and_cluster(validator_pubkey)?; + + let epoch = preferences.proposal_slot.epoch(E::slots_per_epoch()); + let domain = self.get_domain(epoch, Domain::ProposerPreferences); + let signing_root = preferences.signing_root(domain); + + // Envelope slot = the duty's proposal_slot (SIP-94 §5/§7), NOT slot_clock.now() and NOT + // epoch-start. It becomes PartialSignatureMessages.slot on the wire, which peers accept via + // the ProposerPreferences earliness allowance and validate proposer-assignment against + // (#1062). A future proposal_slot also keeps the collector alive until that slot passes. + let proposal_slot = preferences.proposal_slot; + + let future = async { + let signature = match self + .collect_signature( + PartialSignatureKind::ProposerPreferences, + Role::ProposerPreferences, + CollectionMode::SingleValidator, + &validator, + &cluster, + signing_root, + proposal_slot, + ) + .await + { + Ok(signature) => signature, + Err(err) => { + self.report_proposer_preferences_collection_failure( + &err, + &preferences, + signing_root, + ); + return Err(err); + } + }; + + Ok(SignedProposerPreferences { + message: preferences, + signature, + }) + }; + + run_and_update_metrics( + PROPOSER_PREFERENCES_LOG_NAME, + &metrics::SIGNED_PROPOSER_PREFERENCES_TOTAL, + future, + ) + .await } } diff --git a/anchor/validator_store/src/metrics.rs b/anchor/validator_store/src/metrics.rs index 526335a86..6621213e2 100644 --- a/anchor/validator_store/src/metrics.rs +++ b/anchor/validator_store/src/metrics.rs @@ -27,6 +27,15 @@ pub static SIGNED_RANDAO_REVEALS_TOTAL: LazyLock> = LazyLo ) }); +pub static SIGNED_PROPOSER_PREFERENCES_TOTAL: LazyLock> = + LazyLock::new(|| { + try_create_int_counter_vec( + "signed_proposer_preferences_total", + "Total count of ProposerPreferences signings", + &["status"], + ) + }); + // ═══════════════════════════════════════════════════════════════════════════════ // MetadataService metrics // ═══════════════════════════════════════════════════════════════════════════════ @@ -147,3 +156,19 @@ pub static PTC_RECONSTRUCTION_FAILURES: LazyLock> = LazyLo &["reason"], ) }); + +/// Threshold-not-reached for a ProposerPreferences signing. An upper bound on true signing-root +/// divergence: at the current collector granularity it also covers channel close / not-enough +/// operators, which are indistinguishable from a root split at the partial-signature wire. +pub const PROPOSER_PREFERENCES_FAILURE_SIGNING_ROOT_DIVERGENCE: &str = "signing_root_divergence"; +/// Local collection or reconstruction infrastructure fault. +pub const PROPOSER_PREFERENCES_FAILURE_INFRA: &str = "infra"; + +pub static PROPOSER_PREFERENCES_RECONSTRUCTION_FAILURES: LazyLock> = + LazyLock::new(|| { + try_create_int_counter_vec( + "anchor_proposer_preferences_reconstruction_failures_total", + "ProposerPreferences signature collection failures by reason", + &["reason"], + ) + }); diff --git a/anchor/validator_store/src/testing/mod.rs b/anchor/validator_store/src/testing/mod.rs index eb88a968c..6d6a26666 100644 --- a/anchor/validator_store/src/testing/mod.rs +++ b/anchor/validator_store/src/testing/mod.rs @@ -4,3 +4,4 @@ mod committee_aggregate; mod committee_attestation; mod committee_attestation_gloas; mod payload_attestation; +mod proposer_preferences; diff --git a/anchor/validator_store/src/testing/proposer_preferences.rs b/anchor/validator_store/src/testing/proposer_preferences.rs new file mode 100644 index 000000000..6147b6cb1 --- /dev/null +++ b/anchor/validator_store/src/testing/proposer_preferences.rs @@ -0,0 +1,371 @@ +//! Integration tests for the ProposerPreferences path in `sign_proposer_preferences()`. +//! +//! These tests pin the two decisions that are unique to this duty and share no coverage with any +//! other signing path: +//! - the signing domain is keyed by the *proposal* slot's epoch (`Domain::ProposerPreferences`), +//! - the partial-signature collection is scheduled at the future `proposal_slot` itself (SIP-94 +//! §5/§7), not `slot_clock.now()` and not the proposal epoch's start slot. Peers accept the +//! future slot via the ProposerPreferences role's earliness allowance and validate proposer +//! assignment against it (#1062), and keying the collector to `proposal_slot` keeps it alive +//! until `proposal_slot + 1` passes. +use std::sync::LazyLock; + +use bls::FixedBytesExtended; +use signature_collector::{CollectionError, SignatureRequester}; +use ssv_types::{OperatorId, msgid::Role, partial_sig::PartialSignatureKind}; +use types::{ + Address, ChainSpec, Domain, EthSpec, Hash256, MainnetEthSpec, ProposerPreferences, SignedRoot, + Slot, +}; +use validator_store::ValidatorStore; + +use super::common::*; +use crate::{Error, SpecificError}; + +/// Non-zero so a correctly echoed beacon index is distinguishable from an accidental default 0. +const STARTING_VALIDATOR_INDEX: usize = 5; +/// Distinctive fee recipient / gas limit so an echoed message is distinguishable from a default. +const TEST_GAS_LIMIT: u64 = 30_000_000; +/// Number of epochs a lookahead proposal slot sits ahead of the send slot. Two epochs places the +/// proposal slot at a value distinct from both `TEST_SLOT` and the proposal epoch's start slot, so +/// the envelope-slot test can rule out both alternatives. +const LOOKAHEAD_EPOCHS: u64 = 2; + +/// Serializes the two metric tests against each other. Both read the same labels of the global +/// prometheus `PROPOSER_PREFERENCES_RECONSTRUCTION_FAILURES` counter, so concurrent execution +/// would make their cross-label delta assertions racy. A tokio mutex rather than std because the +/// guard is held across awaits on a multi-thread runtime. +static METRIC_TEST_LOCK: LazyLock> = + LazyLock::new(|| tokio::sync::Mutex::new(())); + +fn test_operator_ids() -> [OperatorId; 4] { + [OperatorId(1), OperatorId(2), OperatorId(3), OperatorId(4)] +} + +/// Builds a `ProposerPreferences` fixture. The store signs whatever it is handed, so a fixed +/// fixture is sufficient; `proposal_slot` is a parameter because it keys the signing domain and +/// the envelope-slot behavior the tests assert on. +fn create_proposer_preferences(validator_index: u64, proposal_slot: Slot) -> ProposerPreferences { + ProposerPreferences { + dependent_root: Hash256::from([7u8; 32]), + proposal_slot, + validator_index, + fee_recipient: Address::repeat_byte(0xab), + target_gas_limit: TEST_GAS_LIMIT, + } +} + +/// Independently recomputes the expected signing root for a `ProposerPreferences`, using the same +/// mainnet spec and zero `genesis_validators_root` the harness wires up. Pins that the domain is +/// `Domain::ProposerPreferences` and that its epoch is derived from `proposal_slot`. +fn expected_signing_root(preferences: &ProposerPreferences) -> Hash256 { + let spec = ChainSpec::mainnet(); + let epoch = preferences + .proposal_slot + .epoch(MainnetEthSpec::slots_per_epoch()); + let domain = spec.get_domain( + epoch, + Domain::ProposerPreferences, + &spec.fork_at_epoch(epoch), + Hash256::zero(), + ); + preferences.signing_root(domain) +} + +// ==================== Success / signing-root tests ==================== + +/// `sign_proposer_preferences` collects a single-validator signature and echoes the input back in +/// the resulting message, committing to the `ProposerPreferences` under the `ProposerPreferences` +/// domain keyed by the proposal slot's epoch. +/// +/// Uses a lookahead `proposal_slot` (LOOKAHEAD_EPOCHS ahead of the send slot) so the recomputed +/// domain epoch is `epoch(proposal_slot)` for a *future* slot, not the send slot's epoch: this +/// exercises acceptance criterion "domain epoch equals `epoch(proposal_slot)`" for a real +/// lookahead. It also asserts the broadcast `PartialSignatureMessages.slot` (captured via the +/// collector call) equals `preferences.proposal_slot`, the SIP-94 §5/§7 wire-slot invariant. +#[tokio::test(flavor = "multi_thread")] +async fn proposer_preferences_reconstruction_threshold() { + // Arrange + let our_operator_id = OperatorId(1); + let committee = create_committee_setup(&test_operator_ids(), 1, STARTING_VALIDATOR_INDEX); + let pubkey = committee.validators[0].public_key; + let harness = ValidatorStoreTestHarness::new(vec![committee], our_operator_id); + // A future proposal slot so `epoch(proposal_slot)` differs from the send slot's epoch; the + // independent root recompute then pins the domain epoch to the proposal slot, not the clock. + let future_proposal_slot = + Slot::new(TEST_SLOT + MainnetEthSpec::slots_per_epoch() * LOOKAHEAD_EPOCHS); + let preferences = + create_proposer_preferences(STARTING_VALIDATOR_INDEX as u64, future_proposal_slot); + + // Act + let result = harness + .validator_store + .sign_proposer_preferences(pubkey, preferences.clone()) + .await; + + // Assert + let signed = result.expect("proposer preferences signing should succeed"); + assert_eq!( + signed.message, preferences, + "signed message should echo the input preferences unchanged" + ); + + let captured = harness.captured_calls.lock(); + assert_eq!( + captured.len(), + 1, + "expected exactly one sign_and_collect call" + ); + let call = &captured[0]; + match &call.requester { + SignatureRequester::SingleValidator { + pubkey: requester_pubkey, + } => assert_eq!( + *requester_pubkey, pubkey, + "collection should be requested for the signing validator" + ), + other => panic!("expected SignatureRequester::SingleValidator, got: {other:?}"), + } + assert_eq!( + call.metadata.kind, + PartialSignatureKind::ProposerPreferences, + "partial signature messages should be tagged with the ProposerPreferences kind" + ); + assert_eq!( + call.metadata.role, + Role::ProposerPreferences, + "the network message should be routed under the ProposerPreferences role" + ); + // The broadcast `PartialSignatureMessages.slot` (captured off the collector call) must equal + // `proposal_slot`: SIP-94 §5/§7 pins the wire slot to the slot being proposed, and #1062 + // validates proposer assignment against it. + assert_eq!( + call.metadata.slot, preferences.proposal_slot, + "the broadcast partial-signature slot should equal the proposal slot" + ); + + // Recompute the root independently to lock the ProposerPreferences-specific signing decisions + // that no other test covers: the `Domain::ProposerPreferences` choice, the epoch derived from + // `proposal_slot` (here a future slot, so this pins epoch keying to the proposal, not the + // clock), and the signed object being the `ProposerPreferences` itself. A regression to a + // different domain or a different epoch key fails here. + let expected_root = expected_signing_root(&preferences); + assert_eq!( + call.signing_root, expected_root, + "signing root should commit to the proposer preferences under the ProposerPreferences domain" + ); +} + +/// The partial-signature collection is scheduled at the future `proposal_slot` carried in the +/// body, NOT at the current send slot (`slot_clock.now()`) and NOT at the proposal epoch's start +/// slot. SIP-94 §5/§7 pins the on-wire `PartialSignatureMessages.slot` to `proposal_slot` itself +/// (one runner per proposal slot, matching ssv-spec's `msg.Slot == duty.DutySlot()`). Peers accept +/// the future slot via the ProposerPreferences role's earliness allowance and validate proposer +/// assignment against it (#1062); keying the collector to `proposal_slot` also keeps it alive until +/// `proposal_slot + 1` passes, so lookahead emissions no longer race a 1-slot arrival window. This +/// is the opposite rationale of the earlier send-slot design. +#[tokio::test(flavor = "multi_thread")] +async fn proposer_preferences_envelope_slot_is_proposal_slot() { + // Arrange + let our_operator_id = OperatorId(1); + let committee = create_committee_setup(&test_operator_ids(), 1, STARTING_VALIDATOR_INDEX); + let pubkey = committee.validators[0].public_key; + let harness = ValidatorStoreTestHarness::new(vec![committee], our_operator_id); + + // A proposal LOOKAHEAD_EPOCHS in the future: distinct from the send slot AND from that future + // epoch's start slot, so the assertions below can rule out both alternatives. + let future_proposal_slot = + Slot::new(TEST_SLOT + MainnetEthSpec::slots_per_epoch() * LOOKAHEAD_EPOCHS); + let preferences = + create_proposer_preferences(STARTING_VALIDATOR_INDEX as u64, future_proposal_slot); + + // Act + let result = harness + .validator_store + .sign_proposer_preferences(pubkey, preferences.clone()) + .await; + + // Assert + result.expect("proposer preferences signing should succeed for a future proposal slot"); + + let captured = harness.captured_calls.lock(); + assert_eq!( + captured.len(), + 1, + "expected exactly one sign_and_collect call" + ); + let call = &captured[0]; + + // The envelope slot must be the future `proposal_slot` (SIP-94 §5/§7), so the on-wire + // `PartialSignatureMessages.slot` equals the slot the preference targets. This is what peers + // validate proposer assignment against (#1062) and what keeps the collector alive until + // `proposal_slot + 1`. + assert_eq!( + call.metadata.slot, preferences.proposal_slot, + "collection should be scheduled at the future proposal slot" + ); + // Explicitly rule out the two rejected alternatives: the current send slot and the proposal + // epoch's start slot. `LOOKAHEAD_EPOCHS >= 1` guarantees `proposal_slot` differs from both. + assert_ne!( + call.metadata.slot, + Slot::new(TEST_SLOT), + "the envelope slot must NOT be the current send slot (slot_clock.now())" + ); + let proposal_epoch_start = preferences + .proposal_slot + .epoch(MainnetEthSpec::slots_per_epoch()) + .start_slot(MainnetEthSpec::slots_per_epoch()); + assert_ne!( + call.metadata.slot, proposal_epoch_start, + "the envelope slot must NOT be the proposal epoch's start slot" + ); +} + +// ==================== Failure classification / metrics tests ==================== + +/// A collection failure that classifies as `NoSignature` propagates the real +/// `SignatureCollectionFailed` error (not `Unsupported`) and increments the +/// `signing_root_divergence` reconstruction-failure metric exactly once. +/// +/// Log-field assertion path: the codebase has no tracing/log-capture utility (no `tracing-test` +/// dependency, no `logs_contain`/subscriber-capture helper), so we deliberately do NOT build +/// fragile tracing infrastructure. The warn-log fields (validator index, proposal slot, +/// `target_gas_limit`, `dependent_root`, signing root) and the `signing_root_divergence` metric +/// increment are emitted by the *same* `NoSignature` match arm in +/// `report_proposer_preferences_collection_failure`, so the metric increment proves that warn +/// branch executed; the log-field assertion is covered indirectly. +#[tokio::test(flavor = "multi_thread")] +async fn proposer_preferences_signing_root_divergence_warns_and_metrics() { + // The global prometheus registry makes cross-label delta assertions racy between the two + // metric tests, so they serialize against each other. + let _guard = METRIC_TEST_LOCK.lock().await; + + // Arrange + let our_operator_id = OperatorId(1); + let committee = create_committee_setup(&test_operator_ids(), 1, STARTING_VALIDATOR_INDEX); + let pubkey = committee.validators[0].public_key; + let harness = ValidatorStoreTestHarness::new_with_options( + vec![committee], + our_operator_id, + HarnessOptions { + // QueueClosedError classifies as the NoSignature / signing-root-divergence class. + collector_failure: Some(CollectionError::QueueClosedError), + disable_slashing_protection: true, + }, + ); + let preferences = + create_proposer_preferences(STARTING_VALIDATOR_INDEX as u64, Slot::new(TEST_SLOT)); + + // The metric lives in the global prometheus registry shared by every test in the process, so + // we assert on the delta. The other metric test also touches this label, so the delta is only + // reliable because both tests hold `METRIC_TEST_LOCK`; future failure tests must join that + // serialization or use distinct labels. + let divergence_counter = crate::metrics::PROPOSER_PREFERENCES_RECONSTRUCTION_FAILURES + .as_ref() + .expect("metric should be created") + .with_label_values(&[crate::metrics::PROPOSER_PREFERENCES_FAILURE_SIGNING_ROOT_DIVERGENCE]); + let count_before = divergence_counter.get(); + + // Act + let result = harness + .validator_store + .sign_proposer_preferences(pubkey, preferences) + .await; + + // Assert + assert!( + matches!( + result, + Err(Error::SpecificError( + SpecificError::SignatureCollectionFailed(CollectionError::QueueClosedError) + )) + ), + "expected QueueClosedError surfaced as SignatureCollectionFailed (the real error, not \ + Unsupported), got: {result:?}" + ); + assert_eq!( + divergence_counter.get() - count_before, + 1, + "QueueClosedError should increment the signing_root_divergence reconstruction-failure \ + metric once" + ); +} + +/// The code classifies collection failures only into the coarse `signing_root_divergence` / +/// `infra` buckets; it never attributes a divergence to a specific input field. A +/// `target_gas_limit` or `dependent_root` mismatch is only observable as a signing-root split +/// (threshold-not-reached), which surfaces here as the same `QueueClosedError`. +/// +/// This asserts the `signing_root_divergence` label increments while the per-input attribution +/// labels (`"target_gas_limit_divergence"`, `"dependent_root_divergence"`) stay at 0, proving the +/// code emits no per-input attribution reason. +#[tokio::test(flavor = "multi_thread")] +async fn proposer_preferences_does_not_classify_remote_input_without_metadata() { + // Touches the same global metric as the other failure test, so it joins the same + // serialization. + let _guard = METRIC_TEST_LOCK.lock().await; + + // Arrange + let our_operator_id = OperatorId(1); + let committee = create_committee_setup(&test_operator_ids(), 1, STARTING_VALIDATOR_INDEX); + let pubkey = committee.validators[0].public_key; + let harness = ValidatorStoreTestHarness::new_with_options( + vec![committee], + our_operator_id, + HarnessOptions { + // A target_gas_limit / dependent_root mismatch is only observable as a signing-root + // split, i.e. threshold-not-reached, which the collector surfaces as QueueClosedError. + collector_failure: Some(CollectionError::QueueClosedError), + disable_slashing_protection: true, + }, + ); + let preferences = + create_proposer_preferences(STARTING_VALIDATOR_INDEX as u64, Slot::new(TEST_SLOT)); + + let metric = crate::metrics::PROPOSER_PREFERENCES_RECONSTRUCTION_FAILURES + .as_ref() + .expect("metric should be created"); + let divergence_counter = metric + .with_label_values(&[crate::metrics::PROPOSER_PREFERENCES_FAILURE_SIGNING_ROOT_DIVERGENCE]); + // Read the hypothetical per-input attribution labels directly; the code never emits them, so + // they must stay at zero. Using literal strings (not consts) is deliberate: no such consts + // exist because the production code never references these buckets. + let target_gas_limit_attribution = metric.with_label_values(&["target_gas_limit_divergence"]); + let dependent_root_attribution = metric.with_label_values(&["dependent_root_divergence"]); + let divergence_before = divergence_counter.get(); + + // Act + let result = harness + .validator_store + .sign_proposer_preferences(pubkey, preferences) + .await; + + // Assert + assert!( + matches!( + result, + Err(Error::SpecificError( + SpecificError::SignatureCollectionFailed(CollectionError::QueueClosedError) + )) + ), + "expected QueueClosedError surfaced as SignatureCollectionFailed, got: {result:?}" + ); + assert_eq!( + divergence_counter.get() - divergence_before, + 1, + "the coarse signing_root_divergence bucket should increment once" + ); + // The per-input attribution buckets are never written by the production code: it cannot tell a + // target_gas_limit split from a dependent_root split at the partial-signature wire, so it emits + // no per-input reason. + assert_eq!( + target_gas_limit_attribution.get(), + 0, + "code must not attribute divergence to a target_gas_limit mismatch" + ); + assert_eq!( + dependent_root_attribution.get(), + 0, + "code must not attribute divergence to a dependent_root mismatch" + ); +} From 108a45c6255bc1a930268269fe9c9052a29ce1b7 Mon Sep 17 00:00:00 2001 From: petarjuki7 Date: Wed, 15 Jul 2026 18:24:31 +0200 Subject: [PATCH 2/9] fix(validator_store): relabel ProposerPreferences reconstruction-failure telemetry The NoSignature arm labelled every failed reconstruction signing_root_divergence and warned that operators had likely diverged on the signing root. That class is reached only via QueueClosedError, which conflates threshold-not-reached, too-few operators, and delivery loss, and the wire carries no preference fields, so the collector cannot know a signing-root split occurred. The sibling PTC path already labels the identical class honestly. Rename the label to insufficient_partial_signatures, reword the warn to list signing-root divergence as one of several possible causes, and update the two metric tests. Document CollectionTimeout in the classifier as reserved and not currently produced by the collector. Addresses review feedback on #1125. --- anchor/validator_store/src/instrumentation.rs | 5 +++ anchor/validator_store/src/lib.rs | 8 ++-- anchor/validator_store/src/metrics.rs | 11 +++-- .../src/testing/proposer_preferences.rs | 42 ++++++++++--------- 4 files changed, 40 insertions(+), 26 deletions(-) diff --git a/anchor/validator_store/src/instrumentation.rs b/anchor/validator_store/src/instrumentation.rs index 5882ea5ff..65e064631 100644 --- a/anchor/validator_store/src/instrumentation.rs +++ b/anchor/validator_store/src/instrumentation.rs @@ -51,6 +51,11 @@ pub fn classify_collection_failure(error: &Error) -> CollectionFailureClass { // forces a conscious classification decision here at compile time. Error::SpecificError(SpecificError::SignatureCollectionFailed(collection_error)) => { match collection_error { + // `CollectionTimeout` is not currently produced by the collector (it has no + // deadline path yet); it is classified defensively here alongside the real + // `QueueClosedError` threshold-not-reached signal so that, if a future explicit + // collection deadline starts emitting it, it lands in the same no-signature bucket + // without a silent misclassification. CollectionError::QueueClosedError | CollectionError::CollectionTimeout => { CollectionFailureClass::NoSignature } diff --git a/anchor/validator_store/src/lib.rs b/anchor/validator_store/src/lib.rs index ab07f26c6..349bee32a 100644 --- a/anchor/validator_store/src/lib.rs +++ b/anchor/validator_store/src/lib.rs @@ -924,12 +924,14 @@ impl + 'static> AnchorValidator dependent_root = ?preferences.dependent_root, ?signing_root, ?error, - "ProposerPreferences reconstruction failed; operators likely diverged on the \ - signing root (target_gas_limit or dependent_root)" + "Insufficient partial signatures to reconstruct ProposerPreferences; possible \ + causes: too few operators reached the threshold, partial-signature delivery \ + loss, or operators diverged on the signing root (target_gas_limit or \ + dependent_root)" ); metrics::inc_counter_vec( &metrics::PROPOSER_PREFERENCES_RECONSTRUCTION_FAILURES, - &[metrics::PROPOSER_PREFERENCES_FAILURE_SIGNING_ROOT_DIVERGENCE], + &[metrics::PROPOSER_PREFERENCES_FAILURE_INSUFFICIENT_PARTIAL_SIGNATURES], ); } CollectionFailureClass::Infra => { diff --git a/anchor/validator_store/src/metrics.rs b/anchor/validator_store/src/metrics.rs index 6621213e2..89fc2b004 100644 --- a/anchor/validator_store/src/metrics.rs +++ b/anchor/validator_store/src/metrics.rs @@ -157,10 +157,13 @@ pub static PTC_RECONSTRUCTION_FAILURES: LazyLock> = LazyLo ) }); -/// Threshold-not-reached for a ProposerPreferences signing. An upper bound on true signing-root -/// divergence: at the current collector granularity it also covers channel close / not-enough -/// operators, which are indistinguishable from a root split at the partial-signature wire. -pub const PROPOSER_PREFERENCES_FAILURE_SIGNING_ROOT_DIVERGENCE: &str = "signing_root_divergence"; +/// The committee never reached the partial-signature threshold for a ProposerPreferences signing. +/// At the current collector granularity this single bucket covers every no-quorum cause — too few +/// operators, partial-signature delivery loss, and operators diverging on the signing root +/// (`target_gas_limit` / `dependent_root`) — which are indistinguishable at the wire. Mirrors +/// `PTC_FAILURE_NO_SIGNATURE`. +pub const PROPOSER_PREFERENCES_FAILURE_INSUFFICIENT_PARTIAL_SIGNATURES: &str = + "insufficient_partial_signatures"; /// Local collection or reconstruction infrastructure fault. pub const PROPOSER_PREFERENCES_FAILURE_INFRA: &str = "infra"; diff --git a/anchor/validator_store/src/testing/proposer_preferences.rs b/anchor/validator_store/src/testing/proposer_preferences.rs index 6147b6cb1..344e09aea 100644 --- a/anchor/validator_store/src/testing/proposer_preferences.rs +++ b/anchor/validator_store/src/testing/proposer_preferences.rs @@ -225,17 +225,17 @@ async fn proposer_preferences_envelope_slot_is_proposal_slot() { /// A collection failure that classifies as `NoSignature` propagates the real /// `SignatureCollectionFailed` error (not `Unsupported`) and increments the -/// `signing_root_divergence` reconstruction-failure metric exactly once. +/// `insufficient_partial_signatures` reconstruction-failure metric exactly once. /// /// Log-field assertion path: the codebase has no tracing/log-capture utility (no `tracing-test` /// dependency, no `logs_contain`/subscriber-capture helper), so we deliberately do NOT build /// fragile tracing infrastructure. The warn-log fields (validator index, proposal slot, -/// `target_gas_limit`, `dependent_root`, signing root) and the `signing_root_divergence` metric -/// increment are emitted by the *same* `NoSignature` match arm in +/// `target_gas_limit`, `dependent_root`, signing root) and the `insufficient_partial_signatures` +/// metric increment are emitted by the *same* `NoSignature` match arm in /// `report_proposer_preferences_collection_failure`, so the metric increment proves that warn /// branch executed; the log-field assertion is covered indirectly. #[tokio::test(flavor = "multi_thread")] -async fn proposer_preferences_signing_root_divergence_warns_and_metrics() { +async fn proposer_preferences_insufficient_partial_signatures_warns_and_metrics() { // The global prometheus registry makes cross-label delta assertions racy between the two // metric tests, so they serialize against each other. let _guard = METRIC_TEST_LOCK.lock().await; @@ -248,7 +248,8 @@ async fn proposer_preferences_signing_root_divergence_warns_and_metrics() { vec![committee], our_operator_id, HarnessOptions { - // QueueClosedError classifies as the NoSignature / signing-root-divergence class. + // QueueClosedError classifies as the NoSignature / insufficient-partial-signatures + // class. collector_failure: Some(CollectionError::QueueClosedError), disable_slashing_protection: true, }, @@ -260,11 +261,13 @@ async fn proposer_preferences_signing_root_divergence_warns_and_metrics() { // we assert on the delta. The other metric test also touches this label, so the delta is only // reliable because both tests hold `METRIC_TEST_LOCK`; future failure tests must join that // serialization or use distinct labels. - let divergence_counter = crate::metrics::PROPOSER_PREFERENCES_RECONSTRUCTION_FAILURES + let failure_counter = crate::metrics::PROPOSER_PREFERENCES_RECONSTRUCTION_FAILURES .as_ref() .expect("metric should be created") - .with_label_values(&[crate::metrics::PROPOSER_PREFERENCES_FAILURE_SIGNING_ROOT_DIVERGENCE]); - let count_before = divergence_counter.get(); + .with_label_values(&[ + crate::metrics::PROPOSER_PREFERENCES_FAILURE_INSUFFICIENT_PARTIAL_SIGNATURES, + ]); + let count_before = failure_counter.get(); // Act let result = harness @@ -284,21 +287,21 @@ async fn proposer_preferences_signing_root_divergence_warns_and_metrics() { Unsupported), got: {result:?}" ); assert_eq!( - divergence_counter.get() - count_before, + failure_counter.get() - count_before, 1, - "QueueClosedError should increment the signing_root_divergence reconstruction-failure \ + "QueueClosedError should increment the insufficient_partial_signatures reconstruction-failure \ metric once" ); } -/// The code classifies collection failures only into the coarse `signing_root_divergence` / +/// The code classifies collection failures only into the coarse `insufficient_partial_signatures` / /// `infra` buckets; it never attributes a divergence to a specific input field. A /// `target_gas_limit` or `dependent_root` mismatch is only observable as a signing-root split /// (threshold-not-reached), which surfaces here as the same `QueueClosedError`. /// -/// This asserts the `signing_root_divergence` label increments while the per-input attribution -/// labels (`"target_gas_limit_divergence"`, `"dependent_root_divergence"`) stay at 0, proving the -/// code emits no per-input attribution reason. +/// This asserts the `insufficient_partial_signatures` label increments while the per-input +/// attribution labels (`"target_gas_limit_divergence"`, `"dependent_root_divergence"`) stay at 0, +/// proving the code emits no per-input attribution reason. #[tokio::test(flavor = "multi_thread")] async fn proposer_preferences_does_not_classify_remote_input_without_metadata() { // Touches the same global metric as the other failure test, so it joins the same @@ -325,14 +328,15 @@ async fn proposer_preferences_does_not_classify_remote_input_without_metadata() let metric = crate::metrics::PROPOSER_PREFERENCES_RECONSTRUCTION_FAILURES .as_ref() .expect("metric should be created"); - let divergence_counter = metric - .with_label_values(&[crate::metrics::PROPOSER_PREFERENCES_FAILURE_SIGNING_ROOT_DIVERGENCE]); + let failure_counter = metric.with_label_values(&[ + crate::metrics::PROPOSER_PREFERENCES_FAILURE_INSUFFICIENT_PARTIAL_SIGNATURES, + ]); // Read the hypothetical per-input attribution labels directly; the code never emits them, so // they must stay at zero. Using literal strings (not consts) is deliberate: no such consts // exist because the production code never references these buckets. let target_gas_limit_attribution = metric.with_label_values(&["target_gas_limit_divergence"]); let dependent_root_attribution = metric.with_label_values(&["dependent_root_divergence"]); - let divergence_before = divergence_counter.get(); + let count_before = failure_counter.get(); // Act let result = harness @@ -351,9 +355,9 @@ async fn proposer_preferences_does_not_classify_remote_input_without_metadata() "expected QueueClosedError surfaced as SignatureCollectionFailed, got: {result:?}" ); assert_eq!( - divergence_counter.get() - divergence_before, + failure_counter.get() - count_before, 1, - "the coarse signing_root_divergence bucket should increment once" + "the coarse insufficient_partial_signatures bucket should increment once" ); // The per-input attribution buckets are never written by the production code: it cannot tell a // target_gas_limit split from a dependent_root split at the partial-signature wire, so it emits From ce37a43b2c3976eb20768159529b57134b85ab23 Mon Sep 17 00:00:00 2001 From: petarjuki7 Date: Thu, 16 Jul 2026 11:25:36 +0200 Subject: [PATCH 3/9] test(validator_store): sync ProposerPreferences tests with HarnessOptions epbs (#1082/#1103/#1128) added `spec` and `forced_gloas_index` to the shared test HarnessOptions. Append `..Default::default()` to the two ProposerPreferences failure-test constructions, matching the sibling payload_attestation tests, so the suite compiles on the rebased base. Part of #1063. --- anchor/validator_store/src/testing/proposer_preferences.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/anchor/validator_store/src/testing/proposer_preferences.rs b/anchor/validator_store/src/testing/proposer_preferences.rs index 344e09aea..45b2df6e8 100644 --- a/anchor/validator_store/src/testing/proposer_preferences.rs +++ b/anchor/validator_store/src/testing/proposer_preferences.rs @@ -252,6 +252,7 @@ async fn proposer_preferences_insufficient_partial_signatures_warns_and_metrics( // class. collector_failure: Some(CollectionError::QueueClosedError), disable_slashing_protection: true, + ..Default::default() }, ); let preferences = @@ -320,6 +321,7 @@ async fn proposer_preferences_does_not_classify_remote_input_without_metadata() // split, i.e. threshold-not-reached, which the collector surfaces as QueueClosedError. collector_failure: Some(CollectionError::QueueClosedError), disable_slashing_protection: true, + ..Default::default() }, ); let preferences = From 570129865786fbbb8f2ad69afdd795f4369be7a2 Mon Sep 17 00:00:00 2001 From: petarjuki7 Date: Tue, 21 Jul 2026 00:29:42 +0200 Subject: [PATCH 4/9] fix(validator_store): prefix proposer-preferences signing counter Rename signed_proposer_preferences_total to anchor_signed_proposer_preferences_total so the top-line signing counter carries the same anchor_ prefix as its paired failure counter (anchor_proposer_preferences_reconstruction_failures_total) and the rest of the file. Choosing the prefix before ship avoids a later rename breaking dashboards. Addresses review feedback on #1125. --- anchor/validator_store/src/metrics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/anchor/validator_store/src/metrics.rs b/anchor/validator_store/src/metrics.rs index 89fc2b004..66d702a8c 100644 --- a/anchor/validator_store/src/metrics.rs +++ b/anchor/validator_store/src/metrics.rs @@ -30,7 +30,7 @@ pub static SIGNED_RANDAO_REVEALS_TOTAL: LazyLock> = LazyLo pub static SIGNED_PROPOSER_PREFERENCES_TOTAL: LazyLock> = LazyLock::new(|| { try_create_int_counter_vec( - "signed_proposer_preferences_total", + "anchor_signed_proposer_preferences_total", "Total count of ProposerPreferences signings", &["status"], ) From fd765215e3f52bc9fa1645d9dfd9f11b2322b044 Mon Sep 17 00:00:00 2001 From: petarjuki7 Date: Tue, 21 Jul 2026 00:29:51 +0200 Subject: [PATCH 5/9] fix(validator_store): bound ProposerPreferences collection wait MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wrap the per-validator signature collection in a slot-derived tokio timeout (2 slots) mapped to CollectionError::CollectionTimeout. Without a deadline the await resolves only when the collector is reaped at proposal_slot + 2 (up to ~13 min for a next-epoch lookahead emission); because the LH ProposerPreferencesService awaits each validator sequentially in one task, a single no-quorum validator (an expected SIP-94 §5 state such as a target_gas_limit config mismatch or a dependent_root observation split) blocks both epochs' emissions for that duration. The collector outlives the deadline, so a later per-slot retry still reconstructs if quorum forms. Also drop the run_and_update_metrics wrapper, whose catch-all arm error-logged a no-quorum failure and miscounted it as other_error on top of the reporter's correct warn and insufficient_partial_signatures. Match sign_payload_attestation: the reporter owns failure telemetry, and success is counted explicitly on the Ok path. Addresses review feedback on #1125; closes the bounded-timeout AC of #1063. --- anchor/validator_store/src/lib.rs | 81 ++++++++++++++++++------------- 1 file changed, 48 insertions(+), 33 deletions(-) diff --git a/anchor/validator_store/src/lib.rs b/anchor/validator_store/src/lib.rs index 349bee32a..3f649b928 100644 --- a/anchor/validator_store/src/lib.rs +++ b/anchor/validator_store/src/lib.rs @@ -98,7 +98,10 @@ const AGGREGATE_LOG_NAME: &str = "aggregate"; const SELECTION_PROOF_LOG_NAME: &str = "selection proof"; const SYNC_SELECTION_PROOF_LOG_NAME: &str = "sync selection proof"; const SYNC_COMMITTEE_CONTRIBUTION_LOG_NAME: &str = "sync committee contribution"; -const PROPOSER_PREFERENCES_LOG_NAME: &str = "proposer preferences"; + +/// Upper bound, in slots, on how long `sign_proposer_preferences` waits for a validator's signature +/// to be reconstructed. +const PROPOSER_PREFERENCES_COLLECTION_TIMEOUT_SLOTS: u32 = 2; /// A request to collect a committee signature for a single validator. /// @@ -3552,42 +3555,54 @@ impl + 'static> ValidatorStore // (#1062). A future proposal_slot also keeps the collector alive until that slot passes. let proposal_slot = preferences.proposal_slot; - let future = async { - let signature = match self - .collect_signature( - PartialSignatureKind::ProposerPreferences, - Role::ProposerPreferences, - CollectionMode::SingleValidator, - &validator, - &cluster, - signing_root, - proposal_slot, - ) - .await - { - Ok(signature) => signature, - Err(err) => { - self.report_proposer_preferences_collection_failure( - &err, - &preferences, - signing_root, - ); - return Err(err); - } - }; + // Bound the wait so a no-quorum validator cannot head-of-line-block the LH service's + // sequential per-validator loop (see `PROPOSER_PREFERENCES_COLLECTION_TIMEOUT_SLOTS`). + let collection_timeout = + self.spec.get_slot_duration() * PROPOSER_PREFERENCES_COLLECTION_TIMEOUT_SLOTS; - Ok(SignedProposerPreferences { - message: preferences, - signature, - }) + // Map a deadline elapse to `CollectionTimeout` so it and any collector error share the + // single reporting/return path below. + let collected = match tokio::time::timeout( + collection_timeout, + self.collect_signature( + PartialSignatureKind::ProposerPreferences, + Role::ProposerPreferences, + CollectionMode::SingleValidator, + &validator, + &cluster, + signing_root, + proposal_slot, + ), + ) + .await + { + Ok(result) => result, + Err(_elapsed) => Err(Error::SpecificError( + SpecificError::SignatureCollectionFailed(CollectionError::CollectionTimeout), + )), }; - run_and_update_metrics( - PROPOSER_PREFERENCES_LOG_NAME, + let signature = match collected { + Ok(signature) => signature, + Err(err) => { + self.report_proposer_preferences_collection_failure( + &err, + &preferences, + signing_root, + ); + return Err(err); + } + }; + + validator_metrics::inc_counter_vec( &metrics::SIGNED_PROPOSER_PREFERENCES_TOTAL, - future, - ) - .await + &[validator_metrics::SUCCESS], + ); + + Ok(SignedProposerPreferences { + message: preferences, + signature, + }) } } From 7cb540237a6d3e5121ad1604bdca5fae31270fcb Mon Sep 17 00:00:00 2001 From: petarjuki7 Date: Tue, 21 Jul 2026 01:09:55 +0200 Subject: [PATCH 6/9] test(validator_store): make ProposerPreferences tests falsifiable Three tests asserted conditions that held regardless of the behavior they named, so none could catch its target regression: - the signing-domain recompute ran under ChainSpec::mainnet(), where the send and proposal epochs share the genesis fork version, so keying the domain on the send epoch would produce an identical root; - one test asserted attribution labels the production code never writes, so the zero deltas held even if the whole reporting arm were deleted; - comments claimed the on-wire PartialSignatureMessages.slot was asserted, but the mock captures metadata.slot at the trait boundary before create_message runs. Fix by mirroring the payload_attestation sibling suite: - run the success test on a spec with Gloas activated at the lookahead epoch, so a fork boundary sits between the send and proposal epochs, and assert the root matches the proposal-epoch domain and differs from the send-epoch domain; - drop the vacuous attribution-label test; add an infra-class test (EmptySignature) that gives the Infra classification arm its first coverage, and a zero-delta infra check on the QueueClosedError test; - add a slashing-protection tripwire; - reword the slot comments to state metadata.slot is what is asserted. Addresses review feedback on #1125. --- anchor/validator_store/src/testing/common.rs | 12 + .../src/testing/proposer_preferences.rs | 246 ++++++++++++------ 2 files changed, 185 insertions(+), 73 deletions(-) diff --git a/anchor/validator_store/src/testing/common.rs b/anchor/validator_store/src/testing/common.rs index b85f24e56..e7dd8fb64 100644 --- a/anchor/validator_store/src/testing/common.rs +++ b/anchor/validator_store/src/testing/common.rs @@ -269,6 +269,18 @@ pub(super) fn gloas_at_genesis_spec() -> Arc { Arc::new(spec) } +/// Builds a `ChainSpec` based on mainnet but with Gloas activated at `gloas_epoch` (leaving +/// mainnet's other fork epochs untouched, so they stay far in the future). This places a fork +/// boundary strictly inside a lookahead window: an epoch below `gloas_epoch` resolves to the +/// genesis fork version while `gloas_epoch` and beyond resolve to the Gloas fork version, giving +/// two byte-distinct signing domains on either side of the boundary. Used to make the +/// "domain keyed on the *proposal* epoch, not the send epoch" assertion falsifiable. +pub(super) fn gloas_at_epoch_spec(gloas_epoch: Epoch) -> Arc { + let mut spec = ChainSpec::mainnet(); + spec.gloas_fork_epoch = Some(gloas_epoch); + Arc::new(spec) +} + /// Builds a `ChainSpec` based on mainnet but with Electra activated at genesis and Gloas /// disabled, so `TEST_SLOT` resolves to Electra (post-Electra, pre-Gloas). The committee QBFT /// decides over `BeaconVote` and the attestation index stays untouched at `0`. diff --git a/anchor/validator_store/src/testing/proposer_preferences.rs b/anchor/validator_store/src/testing/proposer_preferences.rs index 45b2df6e8..aa6fd82d2 100644 --- a/anchor/validator_store/src/testing/proposer_preferences.rs +++ b/anchor/validator_store/src/testing/proposer_preferences.rs @@ -8,14 +8,21 @@ //! future slot via the ProposerPreferences role's earliness allowance and validate proposer //! assignment against it (#1062), and keying the collector to `proposal_slot` keeps it alive //! until `proposal_slot + 1` passes. +//! +//! Scope of the slot assertions: these tests assert `call.metadata.slot`, the value captured by +//! the mock at the `sign_and_collect` trait boundary. The real `create_message` that builds the +//! on-wire `PartialSignatureMessages` never runs here, so the wire-slot field is not observed +//! directly. `create_message` copies `metadata.slot` verbatim into `PartialSignatureMessages.slot` +//! (see `signature_collector::SignatureCollectorManager::create_message`), and that verbatim copy +//! is covered by signature_collector's own tests; asserting `metadata.slot` therefore pins the +//! input to that copy. use std::sync::LazyLock; -use bls::FixedBytesExtended; use signature_collector::{CollectionError, SignatureRequester}; use ssv_types::{OperatorId, msgid::Role, partial_sig::PartialSignatureKind}; use types::{ - Address, ChainSpec, Domain, EthSpec, Hash256, MainnetEthSpec, ProposerPreferences, SignedRoot, - Slot, + Address, ChainSpec, Domain, Epoch, EthSpec, Hash256, MainnetEthSpec, ProposerPreferences, + SignedRoot, Slot, }; use validator_store::ValidatorStore; @@ -55,19 +62,26 @@ fn create_proposer_preferences(validator_index: u64, proposal_slot: Slot) -> Pro } } -/// Independently recomputes the expected signing root for a `ProposerPreferences`, using the same -/// mainnet spec and zero `genesis_validators_root` the harness wires up. Pins that the domain is -/// `Domain::ProposerPreferences` and that its epoch is derived from `proposal_slot`. -fn expected_signing_root(preferences: &ProposerPreferences) -> Hash256 { - let spec = ChainSpec::mainnet(); - let epoch = preferences - .proposal_slot - .epoch(MainnetEthSpec::slots_per_epoch()); +/// Independently recomputes the expected signing root for a `ProposerPreferences` under the +/// `Domain::ProposerPreferences` domain keyed at an explicit `epoch`, using the harness's own +/// `spec` and `genesis_validators_root` (rather than hardcoding a spec / zero root) so the +/// recompute exactly tracks the store's fork selection. +/// +/// `epoch` is a parameter, not derived from `proposal_slot`, so a caller can recompute the root +/// under both the proposal epoch (the correct key) and the send epoch (the regressed key) and +/// contrast them. On a spec with a fork boundary between those two epochs the domains differ, which +/// is what makes the "keyed on the proposal epoch, not the send epoch" assertion falsifiable. +fn expected_signing_root( + preferences: &ProposerPreferences, + spec: &ChainSpec, + genesis_validators_root: Hash256, + epoch: Epoch, +) -> Hash256 { let domain = spec.get_domain( epoch, Domain::ProposerPreferences, &spec.fork_at_epoch(epoch), - Hash256::zero(), + genesis_validators_root, ); preferences.signing_root(domain) } @@ -79,19 +93,38 @@ fn expected_signing_root(preferences: &ProposerPreferences) -> Hash256 { /// domain keyed by the proposal slot's epoch. /// /// Uses a lookahead `proposal_slot` (LOOKAHEAD_EPOCHS ahead of the send slot) so the recomputed -/// domain epoch is `epoch(proposal_slot)` for a *future* slot, not the send slot's epoch: this -/// exercises acceptance criterion "domain epoch equals `epoch(proposal_slot)`" for a real -/// lookahead. It also asserts the broadcast `PartialSignatureMessages.slot` (captured via the -/// collector call) equals `preferences.proposal_slot`, the SIP-94 §5/§7 wire-slot invariant. +/// domain epoch is `epoch(proposal_slot)` for a *future* slot, not the send slot's epoch. +/// Critically, the harness runs on a spec with Gloas activated exactly at `LOOKAHEAD_EPOCHS`, so a +/// fork boundary sits strictly between the send epoch (0, genesis fork version) and the proposal +/// epoch (`LOOKAHEAD_EPOCHS`, Gloas fork version). Those two fork versions produce byte-distinct +/// signing domains, so the test can both (a) assert the observed root matches a recompute keyed at +/// the proposal epoch and (b) assert it does NOT match a recompute keyed at the send epoch. +/// Assertion (b) is the falsifiability guard: a regression to keying the domain on the send epoch +/// would flip it, whereas under `ChainSpec::mainnet()` (send and proposal epochs both pre-Altair) +/// both epochs share the genesis fork version and the guard could not distinguish them. +/// +/// It also asserts `call.metadata.slot` (the value captured at the collector boundary, which +/// `create_message` copies verbatim into `PartialSignatureMessages.slot`) equals +/// `preferences.proposal_slot`, the SIP-94 §5/§7 wire-slot invariant. #[tokio::test(flavor = "multi_thread")] async fn proposer_preferences_reconstruction_threshold() { // Arrange let our_operator_id = OperatorId(1); let committee = create_committee_setup(&test_operator_ids(), 1, STARTING_VALIDATOR_INDEX); let pubkey = committee.validators[0].public_key; - let harness = ValidatorStoreTestHarness::new(vec![committee], our_operator_id); - // A future proposal slot so `epoch(proposal_slot)` differs from the send slot's epoch; the - // independent root recompute then pins the domain epoch to the proposal slot, not the clock. + // Gloas at epoch LOOKAHEAD_EPOCHS puts a fork boundary strictly inside the lookahead window, so + // the proposal epoch (Gloas) and the send epoch (genesis) resolve to different signing domains. + let harness = ValidatorStoreTestHarness::new_with_options( + vec![committee], + our_operator_id, + HarnessOptions { + spec: gloas_at_epoch_spec(Epoch::new(LOOKAHEAD_EPOCHS)), + ..Default::default() + }, + ); + // A future proposal slot so `epoch(proposal_slot)` differs from the send slot's epoch; with the + // Gloas boundary at LOOKAHEAD_EPOCHS, `epoch(proposal_slot) == LOOKAHEAD_EPOCHS` is the Gloas + // epoch while the send epoch (0) stays on the genesis fork version. let future_proposal_slot = Slot::new(TEST_SLOT + MainnetEthSpec::slots_per_epoch() * LOOKAHEAD_EPOCHS); let preferences = @@ -136,23 +169,45 @@ async fn proposer_preferences_reconstruction_threshold() { Role::ProposerPreferences, "the network message should be routed under the ProposerPreferences role" ); - // The broadcast `PartialSignatureMessages.slot` (captured off the collector call) must equal - // `proposal_slot`: SIP-94 §5/§7 pins the wire slot to the slot being proposed, and #1062 - // validates proposer assignment against it. + // `call.metadata.slot` (captured at the collector boundary) must equal `proposal_slot`: SIP-94 + // §5/§7 pins the wire slot to the slot being proposed, and #1062 validates proposer assignment + // against it. `create_message` copies this value verbatim into `PartialSignatureMessages.slot`. assert_eq!( call.metadata.slot, preferences.proposal_slot, - "the broadcast partial-signature slot should equal the proposal slot" + "the collected partial-signature slot should equal the proposal slot" ); // Recompute the root independently to lock the ProposerPreferences-specific signing decisions - // that no other test covers: the `Domain::ProposerPreferences` choice, the epoch derived from - // `proposal_slot` (here a future slot, so this pins epoch keying to the proposal, not the - // clock), and the signed object being the `ProposerPreferences` itself. A regression to a - // different domain or a different epoch key fails here. - let expected_root = expected_signing_root(&preferences); + // that no other test covers: the `Domain::ProposerPreferences` choice, the epoch keyed off + // `proposal_slot`, and the signed object being the `ProposerPreferences` itself. Recompute + // under both epochs and assert the observed root matches the proposal epoch but NOT the send + // epoch. The inequality is meaningful only because the Gloas boundary at LOOKAHEAD_EPOCHS makes + // the two epochs' domains differ; a regression to send-epoch keying would satisfy the first + // assert but fail on flipping to match the second recompute. + let slots_per_epoch = MainnetEthSpec::slots_per_epoch(); + let proposal_epoch = preferences.proposal_slot.epoch(slots_per_epoch); + let send_epoch = Slot::new(TEST_SLOT).epoch(slots_per_epoch); + let expected_root = expected_signing_root( + &preferences, + &harness.spec, + harness.genesis_validators_root, + proposal_epoch, + ); assert_eq!( call.signing_root, expected_root, - "signing root should commit to the proposer preferences under the ProposerPreferences domain" + "signing root should commit to the proposer preferences under the ProposerPreferences \ + domain keyed by the proposal slot's epoch" + ); + let send_epoch_root = expected_signing_root( + &preferences, + &harness.spec, + harness.genesis_validators_root, + send_epoch, + ); + assert_ne!( + call.signing_root, send_epoch_root, + "signing root must NOT be keyed by the send slot's epoch: the proposal epoch (Gloas) and \ + the send epoch (genesis) yield different domains, so send-epoch keying is observable here" ); } @@ -196,10 +251,10 @@ async fn proposer_preferences_envelope_slot_is_proposal_slot() { ); let call = &captured[0]; - // The envelope slot must be the future `proposal_slot` (SIP-94 §5/§7), so the on-wire - // `PartialSignatureMessages.slot` equals the slot the preference targets. This is what peers - // validate proposer assignment against (#1062) and what keeps the collector alive until - // `proposal_slot + 1`. + // `call.metadata.slot` (captured at the collector boundary) must be the future `proposal_slot` + // (SIP-94 §5/§7). `create_message` copies it verbatim into `PartialSignatureMessages.slot`, so + // the on-wire slot equals the slot the preference targets: this is what peers validate proposer + // assignment against (#1062) and what keeps the collector alive until `proposal_slot + 1`. assert_eq!( call.metadata.slot, preferences.proposal_slot, "collection should be scheduled at the future proposal slot" @@ -259,16 +314,19 @@ async fn proposer_preferences_insufficient_partial_signatures_warns_and_metrics( create_proposer_preferences(STARTING_VALIDATOR_INDEX as u64, Slot::new(TEST_SLOT)); // The metric lives in the global prometheus registry shared by every test in the process, so - // we assert on the delta. The other metric test also touches this label, so the delta is only + // we assert on deltas. The infra metric test also touches these labels, so the deltas are only // reliable because both tests hold `METRIC_TEST_LOCK`; future failure tests must join that // serialization or use distinct labels. - let failure_counter = crate::metrics::PROPOSER_PREFERENCES_RECONSTRUCTION_FAILURES + let metric = crate::metrics::PROPOSER_PREFERENCES_RECONSTRUCTION_FAILURES .as_ref() - .expect("metric should be created") - .with_label_values(&[ - crate::metrics::PROPOSER_PREFERENCES_FAILURE_INSUFFICIENT_PARTIAL_SIGNATURES, - ]); - let count_before = failure_counter.get(); + .expect("metric should be created"); + let insufficient_counter = metric.with_label_values(&[ + crate::metrics::PROPOSER_PREFERENCES_FAILURE_INSUFFICIENT_PARTIAL_SIGNATURES, + ]); + let infra_counter = + metric.with_label_values(&[crate::metrics::PROPOSER_PREFERENCES_FAILURE_INFRA]); + let insufficient_before = insufficient_counter.get(); + let infra_before = infra_counter.get(); // Act let result = harness @@ -288,25 +346,34 @@ async fn proposer_preferences_insufficient_partial_signatures_warns_and_metrics( Unsupported), got: {result:?}" ); assert_eq!( - failure_counter.get() - count_before, + insufficient_counter.get() - insufficient_before, 1, "QueueClosedError should increment the insufficient_partial_signatures reconstruction-failure \ metric once" ); + // Pins the classification boundary: QueueClosedError is a NoSignature-class failure and must + // NOT be attributed to the infra bucket. A regression that reclassified it as infra would flip + // this zero delta. + assert_eq!( + infra_counter.get() - infra_before, + 0, + "QueueClosedError must not leak into the infra reconstruction-failure metric" + ); } -/// The code classifies collection failures only into the coarse `insufficient_partial_signatures` / -/// `infra` buckets; it never attributes a divergence to a specific input field. A -/// `target_gas_limit` or `dependent_root` mismatch is only observable as a signing-root split -/// (threshold-not-reached), which surfaces here as the same `QueueClosedError`. +/// An infrastructure collection failure (`EmptySignature`) propagates the real +/// `SignatureCollectionFailed` error and increments the `infra` reconstruction-failure metric, +/// leaving the `insufficient_partial_signatures` metric untouched. /// -/// This asserts the `insufficient_partial_signatures` label increments while the per-input -/// attribution labels (`"target_gas_limit_divergence"`, `"dependent_root_divergence"`) stay at 0, -/// proving the code emits no per-input attribution reason. +/// This gives the `Infra` arm of `report_proposer_preferences_collection_failure` its first +/// coverage and pins the classification boundary: the zero delta on +/// `insufficient_partial_signatures` proves an infra failure does not drift into the +/// observation-divergence bucket (whose value is an upper bound on the true divergence rate, so a +/// leak would silently inflate it). #[tokio::test(flavor = "multi_thread")] -async fn proposer_preferences_does_not_classify_remote_input_without_metadata() { - // Touches the same global metric as the other failure test, so it joins the same - // serialization. +async fn proposer_preferences_infra_failure_increments_infra_metric() { + // Reads the same global prometheus labels as the other failure test, so it joins the same + // serialization; the cross-label zero-delta reads are only reliable under this lock. let _guard = METRIC_TEST_LOCK.lock().await; // Arrange @@ -317,9 +384,8 @@ async fn proposer_preferences_does_not_classify_remote_input_without_metadata() vec![committee], our_operator_id, HarnessOptions { - // A target_gas_limit / dependent_root mismatch is only observable as a signing-root - // split, i.e. threshold-not-reached, which the collector surfaces as QueueClosedError. - collector_failure: Some(CollectionError::QueueClosedError), + // EmptySignature classifies as the infra failure class. + collector_failure: Some(CollectionError::EmptySignature), disable_slashing_protection: true, ..Default::default() }, @@ -330,15 +396,13 @@ async fn proposer_preferences_does_not_classify_remote_input_without_metadata() let metric = crate::metrics::PROPOSER_PREFERENCES_RECONSTRUCTION_FAILURES .as_ref() .expect("metric should be created"); - let failure_counter = metric.with_label_values(&[ + let infra_counter = + metric.with_label_values(&[crate::metrics::PROPOSER_PREFERENCES_FAILURE_INFRA]); + let insufficient_counter = metric.with_label_values(&[ crate::metrics::PROPOSER_PREFERENCES_FAILURE_INSUFFICIENT_PARTIAL_SIGNATURES, ]); - // Read the hypothetical per-input attribution labels directly; the code never emits them, so - // they must stay at zero. Using literal strings (not consts) is deliberate: no such consts - // exist because the production code never references these buckets. - let target_gas_limit_attribution = metric.with_label_values(&["target_gas_limit_divergence"]); - let dependent_root_attribution = metric.with_label_values(&["dependent_root_divergence"]); - let count_before = failure_counter.get(); + let infra_before = infra_counter.get(); + let insufficient_before = insufficient_counter.get(); // Act let result = harness @@ -351,27 +415,63 @@ async fn proposer_preferences_does_not_classify_remote_input_without_metadata() matches!( result, Err(Error::SpecificError( - SpecificError::SignatureCollectionFailed(CollectionError::QueueClosedError) + SpecificError::SignatureCollectionFailed(CollectionError::EmptySignature) )) ), - "expected QueueClosedError surfaced as SignatureCollectionFailed, got: {result:?}" + "expected EmptySignature surfaced as SignatureCollectionFailed, got: {result:?}" ); assert_eq!( - failure_counter.get() - count_before, + infra_counter.get() - infra_before, 1, - "the coarse insufficient_partial_signatures bucket should increment once" + "EmptySignature should increment the infra reconstruction-failure metric once" ); - // The per-input attribution buckets are never written by the production code: it cannot tell a - // target_gas_limit split from a dependent_root split at the partial-signature wire, so it emits - // no per-input reason. + // The zero delta pins the classification boundary: an infra variant drifting into the + // insufficient_partial_signatures bucket would silently inflate the divergence estimate. assert_eq!( - target_gas_limit_attribution.get(), + insufficient_counter.get() - insufficient_before, 0, - "code must not attribute divergence to a target_gas_limit mismatch" + "infra failures must not leak into the insufficient_partial_signatures divergence metric" + ); +} + +// ==================== Slashing-protection tests ==================== + +/// `sign_proposer_preferences` succeeds with slashing protection enabled, proving the path never +/// consults the slashing DB. +/// +/// Tripwire mechanism: the harness slashing DB is created empty and no validator is ever +/// registered in it, so any slashing-protection check would fail for an unregistered validator. If +/// such a check were ever added to this code path, this call would flip from Ok to Err, which makes +/// the success assertion a real behavioral assertion rather than a tautology. No metric lock is +/// needed: this test reads no global prometheus labels. +#[tokio::test(flavor = "multi_thread")] +async fn proposer_preferences_does_not_touch_slashing_db() { + // Arrange + let our_operator_id = OperatorId(1); + let committee = create_committee_setup(&test_operator_ids(), 1, STARTING_VALIDATOR_INDEX); + let pubkey = committee.validators[0].public_key; + let harness = ValidatorStoreTestHarness::new_with_options( + vec![committee], + our_operator_id, + HarnessOptions { + collector_failure: None, + disable_slashing_protection: false, + ..Default::default() + }, ); + let preferences = + create_proposer_preferences(STARTING_VALIDATOR_INDEX as u64, Slot::new(TEST_SLOT)); + + // Act + let result = harness + .validator_store + .sign_proposer_preferences(pubkey, preferences.clone()) + .await; + + // Assert + let signed = result.expect("signing should succeed despite slashing protection being enabled"); assert_eq!( - dependent_root_attribution.get(), - 0, - "code must not attribute divergence to a dependent_root mismatch" + signed.message, preferences, + "signed message should echo the input preferences unchanged" ); } From 8eda0efab45e9e2a74626eb7a743001496bc5378 Mon Sep 17 00:00:00 2001 From: petarjuki7 Date: Tue, 21 Jul 2026 01:09:55 +0200 Subject: [PATCH 7/9] docs(validator_store): correct CollectionTimeout classifier note sign_proposer_preferences now synthesizes CollectionTimeout when its bounded collection-wait deadline elapses, so the note claiming the variant is not produced and has no deadline path is stale. Reword it to describe the caller that produces it. --- anchor/validator_store/src/instrumentation.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/anchor/validator_store/src/instrumentation.rs b/anchor/validator_store/src/instrumentation.rs index 65e064631..7c00c9b4a 100644 --- a/anchor/validator_store/src/instrumentation.rs +++ b/anchor/validator_store/src/instrumentation.rs @@ -51,11 +51,11 @@ pub fn classify_collection_failure(error: &Error) -> CollectionFailureClass { // forces a conscious classification decision here at compile time. Error::SpecificError(SpecificError::SignatureCollectionFailed(collection_error)) => { match collection_error { - // `CollectionTimeout` is not currently produced by the collector (it has no - // deadline path yet); it is classified defensively here alongside the real - // `QueueClosedError` threshold-not-reached signal so that, if a future explicit - // collection deadline starts emitting it, it lands in the same no-signature bucket - // without a silent misclassification. + // `CollectionTimeout` is synthesized by `sign_proposer_preferences` when its + // bounded collection-wait deadline elapses; the collector itself never emits it. It + // shares the no-signature bucket with the `QueueClosedError` threshold-not-reached + // signal because a deadline elapse likewise means the threshold was not reached + // within the wait window. CollectionError::QueueClosedError | CollectionError::CollectionTimeout => { CollectionFailureClass::NoSignature } From 45cc272a3880703fe372744404ec7409466236d5 Mon Sep 17 00:00:00 2001 From: petarjuki7 Date: Tue, 21 Jul 2026 01:24:37 +0200 Subject: [PATCH 8/9] test(validator_store): cover the ProposerPreferences bounded-timeout path Exercise issue #1063 AC7: a no-quorum collection must fail per-validator with a bounded timeout and never hang the caller. Add a mock-collector hang mode that returns a never-resolving future, and a test that drives the production tokio::time::timeout to elapse under paused virtual time (start_paused), asserting the result is SignatureCollectionFailed(CollectionTimeout) and that it lands in the insufficient_partial_signatures bucket, not infra. Without the production timeout the test would hang on the pending collector future rather than return, which is the "never hangs the caller" property AC7 requires. --- anchor/validator_store/src/testing/common.rs | 28 +++++- .../src/testing/proposer_preferences.rs | 95 +++++++++++++++++++ 2 files changed, 119 insertions(+), 4 deletions(-) diff --git a/anchor/validator_store/src/testing/common.rs b/anchor/validator_store/src/testing/common.rs index e7dd8fb64..79d7e8e26 100644 --- a/anchor/validator_store/src/testing/common.rs +++ b/anchor/validator_store/src/testing/common.rs @@ -125,10 +125,16 @@ pub(super) struct CapturedSignatureCall { pub(super) signing_root: Hash256, } -/// Mock that captures calls and returns a canned infinity signature, or a configured failure. +/// Mock that captures calls and returns a canned infinity signature, or a configured failure, or +/// a future that never resolves (`hang`). struct MockSignatureCollector { captured: CapturedCalls, failure: Option, + /// When `true`, `sign_and_collect` captures the call and then returns a future that never + /// resolves, modeling a quorum that never forms. This lets tests drive the production + /// `tokio::time::timeout` in `sign_proposer_preferences` to elapse; `hang` takes precedence + /// over `failure`. + hang: bool, } impl SignatureCollecting for MockSignatureCollector { @@ -138,13 +144,19 @@ impl SignatureCollecting for MockSignatureCollector { requester: SignatureRequester, signing_data: ValidatorSigningData, ) -> Pin, CollectionError>> + Send + '_>> { - // Capture before failing so tests can assert that the collection attempt happened even - // when the configured outcome is an error. + // Capture before hanging/failing so tests can assert that the collection attempt happened + // even when the configured outcome is a stall or an error. self.captured.lock().push(CapturedSignatureCall { requester, metadata, signing_root: signing_data.root, }); + if self.hang { + // Never resolves, so the caller only unblocks via the production collection timeout. + // `std::future::pending::, CollectionError>>()` is `Send`, which + // satisfies the returned future's bound. + return Box::pin(std::future::pending()); + } if let Some(failure) = self.failure.clone() { return Box::pin(async move { Err(failure) }); } @@ -156,11 +168,13 @@ impl SignatureCollecting for MockSignatureCollector { /// Creates a mock signature collector and returns the shared captured calls handle. fn create_mock_collector( failure: Option, + hang: bool, ) -> (Box, CapturedCalls) { let captured: CapturedCalls = Arc::new(Mutex::new(Vec::new())); let mock = MockSignatureCollector { captured: Arc::clone(&captured), failure, + hang, }; (Box::new(mock), captured) } @@ -238,6 +252,10 @@ pub(super) fn create_committee_setup( pub(super) struct HarnessOptions { /// When set, every `sign_and_collect` call fails with this error after being captured. pub(super) collector_failure: Option, + /// When `true`, every `sign_and_collect` call captures the call and then returns a future that + /// never resolves, modeling a quorum that never forms. Used to drive the production + /// collection-timeout path. Takes precedence over `collector_failure`. + pub(super) collector_hangs: bool, pub(super) disable_slashing_protection: bool, /// Chain spec to wire into the store. Defaults to `ChainSpec::mainnet()`, under which /// `TEST_SLOT` is pre-Electra. Tests that need a specific fork at `TEST_SLOT` (Electra or @@ -252,6 +270,7 @@ impl Default for HarnessOptions { fn default() -> Self { Self { collector_failure: None, + collector_hangs: false, // Slashing protection is disabled by default because the harness never registers // validators in the slashing DB, which would fail block/attestation signing paths. disable_slashing_protection: true, @@ -337,7 +356,8 @@ impl ValidatorStoreTestHarness { "test", )); - let (mock_collector, captured_calls) = create_mock_collector(options.collector_failure); + let (mock_collector, captured_calls) = + create_mock_collector(options.collector_failure, options.collector_hangs); // Database let database = Arc::new( diff --git a/anchor/validator_store/src/testing/proposer_preferences.rs b/anchor/validator_store/src/testing/proposer_preferences.rs index aa6fd82d2..1bdd7ce20 100644 --- a/anchor/validator_store/src/testing/proposer_preferences.rs +++ b/anchor/validator_store/src/testing/proposer_preferences.rs @@ -434,6 +434,101 @@ async fn proposer_preferences_infra_failure_increments_infra_metric() { ); } +/// A collection that never reaches quorum must fail per-validator with a *bounded* timeout and must +/// never hang the caller indefinitely (issue #1063 AC7). The mock collector captures the call and +/// then returns a future that never resolves, so the only way `sign_proposer_preferences` can return +/// is the production `tokio::time::timeout` elapsing after +/// `spec.get_slot_duration() * PROPOSER_PREFERENCES_COLLECTION_TIMEOUT_SLOTS` (= 24s under the +/// harness's mainnet spec). On elapse it synthesizes +/// `CollectionError::CollectionTimeout`, which classifies as the NoSignature bucket and increments +/// the `insufficient_partial_signatures` reconstruction-failure metric. +/// +/// Falsifiability guard (intrinsic, no extra assertion needed): were the production +/// `tokio::time::timeout` removed, this test would hang forever awaiting the pending collector +/// future instead of returning `CollectionTimeout`. Completing at all — and returning the timeout +/// error — is exactly the "never hangs the caller" behavior AC7 requires. +/// +/// Timing: `#[tokio::test(start_paused = true)]` runs on the current-thread runtime with a paused, +/// auto-advancing clock. Neither the harness constructor nor the `sign_proposer_preferences` path +/// spawns a background task that keeps the runtime busy, so once the call awaits the timeout the +/// runtime goes idle and tokio auto-advances virtual time straight to the 24s deadline. The 24s +/// therefore elapse in ~0 real time (verified via a wall-clock guard on the suite run), so no manual +/// `tokio::time::advance` is required. +#[tokio::test(start_paused = true)] +async fn proposer_preferences_no_quorum_hits_bounded_timeout() { + // Reads the same global prometheus labels as the other failure tests, so it joins the same + // serialization; the cross-label delta reads are only reliable under this lock. + let _guard = METRIC_TEST_LOCK.lock().await; + + // Arrange + let our_operator_id = OperatorId(1); + let committee = create_committee_setup(&test_operator_ids(), 1, STARTING_VALIDATOR_INDEX); + let pubkey = committee.validators[0].public_key; + let harness = ValidatorStoreTestHarness::new_with_options( + vec![committee], + our_operator_id, + HarnessOptions { + // The collector captures the call and then never resolves, so only the production + // collection timeout can unblock the caller. + collector_hangs: true, + disable_slashing_protection: true, + ..Default::default() + }, + ); + let preferences = + create_proposer_preferences(STARTING_VALIDATOR_INDEX as u64, Slot::new(TEST_SLOT)); + + let metric = crate::metrics::PROPOSER_PREFERENCES_RECONSTRUCTION_FAILURES + .as_ref() + .expect("metric should be created"); + let insufficient_counter = metric.with_label_values(&[ + crate::metrics::PROPOSER_PREFERENCES_FAILURE_INSUFFICIENT_PARTIAL_SIGNATURES, + ]); + let infra_counter = + metric.with_label_values(&[crate::metrics::PROPOSER_PREFERENCES_FAILURE_INFRA]); + let insufficient_before = insufficient_counter.get(); + let infra_before = infra_counter.get(); + + // Act + let result = harness + .validator_store + .sign_proposer_preferences(pubkey, preferences) + .await; + + // Assert + assert!( + matches!( + result, + Err(Error::SpecificError( + SpecificError::SignatureCollectionFailed(CollectionError::CollectionTimeout) + )) + ), + "a no-quorum collection must surface CollectionTimeout via the bounded collection timeout, \ + got: {result:?}" + ); + assert_eq!( + insufficient_counter.get() - insufficient_before, + 1, + "CollectionTimeout should increment the insufficient_partial_signatures reconstruction-failure \ + metric once (NoSignature bucket)" + ); + // Pins the classification boundary: a bounded-timeout no-quorum is a NoSignature-class failure + // and must NOT be attributed to the infra bucket. + assert_eq!( + infra_counter.get() - infra_before, + 0, + "CollectionTimeout must not leak into the infra reconstruction-failure metric" + ); + // The collection attempt must have started (the call was captured) before the collector hung, + // proving the timeout wrapped an in-flight collection rather than short-circuiting earlier. + let captured = harness.captured_calls.lock(); + assert_eq!( + captured.len(), + 1, + "exactly one sign_and_collect call should have been captured before the collector hung" + ); +} + // ==================== Slashing-protection tests ==================== /// `sign_proposer_preferences` succeeds with slashing protection enabled, proving the path never From 3aeb9083788258f079e959dd54d9df587cfff777 Mon Sep 17 00:00:00 2001 From: shane-moore Date: Mon, 20 Jul 2026 18:30:08 -0700 Subject: [PATCH 9/9] style(validator_store): satisfy rustfmt in bounded-timeout test docs --- .../validator_store/src/testing/proposer_preferences.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/anchor/validator_store/src/testing/proposer_preferences.rs b/anchor/validator_store/src/testing/proposer_preferences.rs index 1bdd7ce20..acfd970b2 100644 --- a/anchor/validator_store/src/testing/proposer_preferences.rs +++ b/anchor/validator_store/src/testing/proposer_preferences.rs @@ -436,8 +436,8 @@ async fn proposer_preferences_infra_failure_increments_infra_metric() { /// A collection that never reaches quorum must fail per-validator with a *bounded* timeout and must /// never hang the caller indefinitely (issue #1063 AC7). The mock collector captures the call and -/// then returns a future that never resolves, so the only way `sign_proposer_preferences` can return -/// is the production `tokio::time::timeout` elapsing after +/// then returns a future that never resolves, so the only way `sign_proposer_preferences` can +/// return is the production `tokio::time::timeout` elapsing after /// `spec.get_slot_duration() * PROPOSER_PREFERENCES_COLLECTION_TIMEOUT_SLOTS` (= 24s under the /// harness's mainnet spec). On elapse it synthesizes /// `CollectionError::CollectionTimeout`, which classifies as the NoSignature bucket and increments @@ -452,8 +452,8 @@ async fn proposer_preferences_infra_failure_increments_infra_metric() { /// auto-advancing clock. Neither the harness constructor nor the `sign_proposer_preferences` path /// spawns a background task that keeps the runtime busy, so once the call awaits the timeout the /// runtime goes idle and tokio auto-advances virtual time straight to the 24s deadline. The 24s -/// therefore elapse in ~0 real time (verified via a wall-clock guard on the suite run), so no manual -/// `tokio::time::advance` is required. +/// therefore elapse in ~0 real time (verified via a wall-clock guard on the suite run), so no +/// manual `tokio::time::advance` is required. #[tokio::test(start_paused = true)] async fn proposer_preferences_no_quorum_hits_bounded_timeout() { // Reads the same global prometheus labels as the other failure tests, so it joins the same