diff --git a/anchor/validator_store/src/instrumentation.rs b/anchor/validator_store/src/instrumentation.rs index 2e3338f36..7c00c9b4a 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,27 @@ 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 { + // `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 => { - 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 3a4c1edd3..203df9db4 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. /// @@ -99,6 +99,10 @@ 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"; +/// 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. /// /// The shared fields (`validator`, `signing_root`) drive `collect_prepared_signatures`, @@ -871,8 +875,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 +888,7 @@ impl + 'static> AnchorValidator &[metrics::PTC_FAILURE_NO_SIGNATURE], ); } - PtcFailureClass::Infra => { + CollectionFailureClass::Infra => { error!( ?validator_pubkey, %slot, @@ -896,7 +900,7 @@ impl + 'static> AnchorValidator &[metrics::PTC_FAILURE_INFRA], ); } - PtcFailureClass::NonCollection => { + CollectionFailureClass::NonCollection => { error!( ?validator_pubkey, %slot, @@ -907,6 +911,55 @@ 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, + "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_INSUFFICIENT_PARTIAL_SIGNATURES], + ); + } + 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, @@ -3488,11 +3541,69 @@ 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; + + // 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; + + // 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), + )), + }; + + 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, + &[validator_metrics::SUCCESS], + ); + + Ok(SignedProposerPreferences { + message: preferences, + signature, + }) } } diff --git a/anchor/validator_store/src/metrics.rs b/anchor/validator_store/src/metrics.rs index 526335a86..66d702a8c 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( + "anchor_signed_proposer_preferences_total", + "Total count of ProposerPreferences signings", + &["status"], + ) + }); + // ═══════════════════════════════════════════════════════════════════════════════ // MetadataService metrics // ═══════════════════════════════════════════════════════════════════════════════ @@ -147,3 +156,22 @@ pub static PTC_RECONSTRUCTION_FAILURES: LazyLock> = LazyLo &["reason"], ) }); + +/// 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"; + +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/common.rs b/anchor/validator_store/src/testing/common.rs index b85f24e56..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, @@ -269,6 +288,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`. @@ -325,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/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..acfd970b2 --- /dev/null +++ b/anchor/validator_store/src/testing/proposer_preferences.rs @@ -0,0 +1,572 @@ +//! 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. +//! +//! 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 signature_collector::{CollectionError, SignatureRequester}; +use ssv_types::{OperatorId, msgid::Role, partial_sig::PartialSignatureKind}; +use types::{ + Address, ChainSpec, Domain, Epoch, 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` 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), + genesis_validators_root, + ); + 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. +/// 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; + // 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 = + 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" + ); + // `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 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 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 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" + ); +} + +/// 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]; + + // `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" + ); + // 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 +/// `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 `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_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; + + // 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 / insufficient-partial-signatures + // class. + collector_failure: Some(CollectionError::QueueClosedError), + disable_slashing_protection: true, + ..Default::default() + }, + ); + 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 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 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::QueueClosedError) + )) + ), + "expected QueueClosedError surfaced as SignatureCollectionFailed (the real error, not \ + Unsupported), got: {result:?}" + ); + assert_eq!( + 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" + ); +} + +/// 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 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_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 + 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 { + // EmptySignature classifies as the infra failure class. + collector_failure: Some(CollectionError::EmptySignature), + 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 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, + ]); + let infra_before = infra_counter.get(); + let insufficient_before = insufficient_counter.get(); + + // Act + let result = harness + .validator_store + .sign_proposer_preferences(pubkey, preferences) + .await; + + // Assert + assert!( + matches!( + result, + Err(Error::SpecificError( + SpecificError::SignatureCollectionFailed(CollectionError::EmptySignature) + )) + ), + "expected EmptySignature surfaced as SignatureCollectionFailed, got: {result:?}" + ); + assert_eq!( + infra_counter.get() - infra_before, + 1, + "EmptySignature should increment the infra reconstruction-failure metric once" + ); + // 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!( + insufficient_counter.get() - insufficient_before, + 0, + "infra failures must not leak into the insufficient_partial_signatures divergence 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 +/// 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!( + signed.message, preferences, + "signed message should echo the input preferences unchanged" + ); +}