From 3bb868ed36b2d0b64b3d8b5448f93921a9191c92 Mon Sep 17 00:00:00 2001 From: shane-moore Date: Wed, 2 Sep 2026 10:58:55 -0700 Subject: [PATCH 1/2] feat(validator_store): detach non-builder envelope signing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sign a non-builder's SIP-94 §6 envelope share from a task spawned in sign_block once the decided block is threshold-signed, instead of from Lighthouse's envelope callback. That callback first fetches the operator's own envelope from its beacon node and never reaches the store when the node holds none (its local bid was external while the cluster decided a self-build block), so the share was lost. The callback now returns a delegated sentinel for non-builder contexts; the builder path is unchanged. Tracks sigp/anchor#1287. --- anchor/validator_store/src/lib.rs | 298 ++++++++++------ .../src/testing/envelope_signing.rs | 324 ++++++++++++++++-- 2 files changed, 490 insertions(+), 132 deletions(-) diff --git a/anchor/validator_store/src/lib.rs b/anchor/validator_store/src/lib.rs index 7a44ae44a..5f39af2b1 100644 --- a/anchor/validator_store/src/lib.rs +++ b/anchor/validator_store/src/lib.rs @@ -9,7 +9,7 @@ use std::{ future::Future, num::NonZeroUsize, str::from_utf8, - sync::{Arc, LazyLock}, + sync::{Arc, LazyLock, Weak}, time::Duration, }; @@ -324,6 +324,9 @@ pub struct AnchorValidatorStore< strict_mfp: bool, is_synced: watch::Receiver, task_executor: TaskExecutor, + /// Self-reference for work that outlives a `&self` trait call: the detached non-builder + /// envelope signing task spawned from [`Self::sign_block`]. + weak_self: Weak, /// `(committee, slot)` keys whose Boole+ `AggregatorCommittee` post-consensus execution has /// already been started. /// @@ -508,7 +511,7 @@ impl + 'static> AnchorValidator is_synced: watch::Receiver, task_executor: TaskExecutor, ) -> Arc> { - Arc::new(Self { + Arc::new_cyclic(|weak_self| Self { database, decrypted_keys: Mutex::new(LruCache::new(MAX_VALIDATORS_PER_OPERATOR)), decided_block_contexts: Mutex::new(HashMap::new()), @@ -533,6 +536,7 @@ impl + 'static> AnchorValidator strict_mfp, is_synced, task_executor, + weak_self: weak_self.clone(), aggregator_post_consensus: Mutex::new(HashSet::new()), }) } @@ -1085,6 +1089,126 @@ impl + 'static> AnchorValidator )) } + /// The payload-due deadline for `slot` (50% of the slot, SIP-94 §6). Past it the envelope + /// cannot satisfy the slot, so neither dissemination nor collection may start or continue. + fn envelope_deadline(&self, slot: Slot) -> Result { + let deadline = self.get_instant_in_slot(slot, self.spec.get_slot_duration() / 2)?; + if Instant::now() >= deadline { + return Err(Error::SpecificError( + SpecificError::EnvelopeDeadlinePassed { slot }, + )); + } + Ok(deadline) + } + + /// Sign another operator's envelope for `(validator, slot)`: the SIP-94 §6 non-builder path. + /// + /// Spawned by [`Self::sign_block`] once the decided block is threshold-signed, when the + /// decided bid is self-build and the decided block is not this operator's own proposal. It + /// runs detached from Lighthouse's envelope callback on purpose: that callback first fetches + /// this operator's own envelope from its beacon node and never reaches the store when the + /// node holds none (its local bid was external), so a non-builder share must not depend on + /// it. Awaits the builder operator's dissemination until the payload-due deadline, validates + /// it against the decided context, and contributes this operator's partial signature. + /// Publishes nothing: only the builder operator holds the payload bytes. + pub(crate) async fn sign_disseminated_envelope( + self: Arc, + validator: ValidatorMetadata, + cluster: Cluster, + context: DecidedBlockContext, + slot: Slot, + ) -> Result<(), Error> { + let record_outcome = |outcome: &str| { + metrics::inc_counter_vec(&metrics::ENVELOPE_SIGNING_OUTCOMES, &[outcome]); + }; + let deadline = self + .envelope_deadline(slot) + .inspect_err(|_| record_outcome(metrics::ENVELOPE_OUTCOME_FAILED))?; + + let Some(dissemination) = self + .dissemination_store + .wait(validator.public_key, slot, deadline) + .await + else { + record_outcome(metrics::ENVELOPE_OUTCOME_FAILED); + return Err(Error::SpecificError(SpecificError::DisseminationTimeout { + slot, + })); + }; + let disseminated = dissemination.blinded_envelope::().map_err(|err| { + record_outcome(metrics::ENVELOPE_OUTCOME_FAILED); + Error::SpecificError(SpecificError::DisseminationUndecodable(err)) + })?; + context.validate_blinded(&disseminated).map_err(|err| { + record_outcome(metrics::ENVELOPE_OUTCOME_FAILED); + Error::SpecificError(err) + })?; + + // `payload_root` is trusted from the builder operator by design (SIP-94 §6). + let domain_hash = self.get_domain(slot.epoch(E::slots_per_epoch()), Domain::BeaconBuilder); + let signing_root = disseminated.signing_root(domain_hash); + let remaining = deadline.saturating_duration_since(Instant::now()); + Self::collect_within( + remaining, + self.collect_signature( + PartialSignatureKind::Envelope, + Role::EnvelopeProposer, + CollectionMode::SingleValidator, + &validator, + &cluster, + signing_root, + slot, + ), + ) + .await + .inspect_err(|_| record_outcome(metrics::ENVELOPE_OUTCOME_FAILED))?; + + info!( + %slot, + validator_pubkey = %validator.public_key, + disseminated_root = ?disseminated.tree_hash_root(), + "Signed another operator's envelope" + ); + record_outcome(metrics::ENVELOPE_OUTCOME_NOT_BUILT_LOCALLY); + Ok(()) + } + + /// Start [`Self::sign_disseminated_envelope`] for `(validator, slot)` when the recorded + /// decision calls for it: a self-build bid on a block another operator built. No-op + /// otherwise (no Gloas context recorded, external build, or this operator is the builder and + /// signs from Lighthouse's envelope callback). Detached; a terminal failure is logged here + /// because nothing awaits the task. + fn spawn_non_builder_envelope_signing( + &self, + validator: &ValidatorMetadata, + cluster: &Cluster, + slot: Slot, + ) { + let Ok(context) = self.get_decided_block_context(validator.public_key, slot) else { + return; + }; + if context.builder_index != BUILDER_INDEX_SELF_BUILD || context.built_locally { + return; + } + let Some(store) = self.weak_self.upgrade() else { + return; + }; + let validator = validator.clone(); + let cluster = cluster.clone(); + let validator_pubkey = validator.public_key; + self.task_executor.spawn( + async move { + if let Err(error) = store + .sign_disseminated_envelope(validator, cluster, context, slot) + .await + { + warn!(?error, %slot, %validator_pubkey, "Non-builder envelope signing failed"); + } + }, + "envelope_non_builder_signing", + ); + } + async fn sign_abstract_block( &self, validator: &ValidatorMetadata, @@ -2934,11 +3058,12 @@ pub enum SpecificError { EnvelopeExternalBuild { builder_index: u64, }, - /// The builder operator disseminated an envelope this operator did not build. This is an - /// intentional non-publish, not a failure. - EnvelopeNotBuiltLocally { - local_root: Hash256, - disseminated_root: Hash256, + /// Another operator built the decided block. This operator's envelope share is signed by + /// the detached non-builder task started from `sign_block`, not from Lighthouse's envelope + /// callback, which has nothing to publish here. This is an intentional non-publish, not a + /// failure. + EnvelopeNonBuilderDelegated { + slot: Slot, }, /// The envelope duty started at or after the payload-due deadline (50% of the slot), past /// which the envelope cannot satisfy this slot. @@ -3331,6 +3456,8 @@ impl + 'static> ValidatorStore "Block threshold signature completed" ); + self.spawn_non_builder_envelope_signing(&validator, &cluster, blinded_block.slot()); + let publish_decision = select_publish_block(signed_block, &blinded_block, local_full_block); Span::current().record("proposal_matched", publish_decision.proposal_matched); @@ -3987,100 +4114,76 @@ impl + 'static> ValidatorStore })); } - // One absolute deadline at the payload-due mark (50% of the slot, SIP-94 §6): - // past it the envelope cannot satisfy this slot, so neither dissemination nor - // collection should proceed or continue. + // Another operator built the decided block. Its envelope reaches this operator by + // dissemination and is signed by the task `sign_block` spawned once the block was + // threshold-signed (`sign_disseminated_envelope`); that task needs nothing from this + // callback, which Lighthouse only reaches after fetching this operator's own envelope + // from its beacon node. The caller publishes every `Ok`, and there is nothing to + // publish, so return the sentinel (SIP-94 §6). + if !context.built_locally { + info!( + "Decided block was built by another operator, its envelope is signed by the non-builder task (expected)" + ); + return Err(Error::SpecificError( + SpecificError::EnvelopeNonBuilderDelegated { slot }, + )); + } + let deadline = self - .get_instant_in_slot(slot, self.spec.get_slot_duration() / 2) + .envelope_deadline(slot) .inspect_err(|_| record_outcome(metrics::ENVELOPE_OUTCOME_FAILED))?; - if Instant::now() >= deadline { + + // The builder operator disseminates its own blinded envelope and signs it; the + // other operators sign the disseminated copy from their non-builder task (SIP-94 §6). + // Bind the local BN's envelope to the decided bid before disseminating: a + // stale or inconsistent BN response must not go out under our signature. + if envelope.payload.block_hash != context.block_hash { + warn!( + local = ?envelope.payload.block_hash, + decided = ?context.block_hash, + "Local envelope's execution block hash differs from the decided bid" + ); record_outcome(metrics::ENVELOPE_OUTCOME_FAILED); return Err(Error::SpecificError( - SpecificError::EnvelopeDeadlinePassed { slot }, + SpecificError::EnvelopeBuilderInconsistent { + local: envelope.payload.block_hash, + decided: context.block_hash, + }, )); } + context.validate_blinded(&local_blinded).map_err(|err| { + warn!(?err, "Local envelope failed the decision bindings"); + record_outcome(metrics::ENVELOPE_OUTCOME_FAILED); + Error::SpecificError(err) + })?; - // The value every operator signs: the builder operator disseminates its own - // blinded envelope; everyone else awaits and validates the disseminated one - // (SIP-94 §6). - let signed_blinded = if context.built_locally { - // Bind the local BN's envelope to the decided bid before disseminating: a - // stale or inconsistent BN response must not go out under our signature. - if envelope.payload.block_hash != context.block_hash { - warn!( - local = ?envelope.payload.block_hash, - decided = ?context.block_hash, - "Local envelope's execution block hash differs from the decided bid" - ); - record_outcome(metrics::ENVELOPE_OUTCOME_FAILED); - return Err(Error::SpecificError( - SpecificError::EnvelopeBuilderInconsistent { - local: envelope.payload.block_hash, - decided: context.block_hash, - }, - )); - } - context.validate_blinded(&local_blinded).map_err(|err| { - warn!(?err, "Local envelope failed the decision bindings"); - record_outcome(metrics::ENVELOPE_OUTCOME_FAILED); - Error::SpecificError(err) - })?; - - let dissemination = EnvelopeDissemination { - slot, - envelope: try_to_variable_list( - local_blinded.as_ssz_bytes(), - |provided, max| { - record_outcome(metrics::ENVELOPE_OUTCOME_FAILED); - Error::SpecificError(SpecificError::DataTooLarge(format!( - "Envelope too large for dissemination: {provided} > {max}" - ))) - }, - )?, - }; - self.signature_collector - .broadcast_dissemination( - validator_pubkey, - cluster.committee_id(), - dissemination, - ) - .map_err(|err| { - warn!(?err, "Envelope dissemination broadcast failed"); + let dissemination = EnvelopeDissemination { + slot, + envelope: try_to_variable_list( + local_blinded.as_ssz_bytes(), + |provided, max| { record_outcome(metrics::ENVELOPE_OUTCOME_FAILED); - Error::SpecificError(SpecificError::DisseminationBroadcastFailed(err)) - })?; - local_blinded.clone() - } else { - let Some(dissemination) = self - .dissemination_store - .wait(validator_pubkey, slot, deadline) - .await - else { - warn!("No envelope dissemination arrived before the payload-due deadline"); - record_outcome(metrics::ENVELOPE_OUTCOME_FAILED); - return Err(Error::SpecificError(SpecificError::DisseminationTimeout { - slot, - })); - }; - let disseminated = dissemination.blinded_envelope::().map_err(|err| { - warn!(?err, "Disseminated envelope bytes did not decode"); - record_outcome(metrics::ENVELOPE_OUTCOME_FAILED); - Error::SpecificError(SpecificError::DisseminationUndecodable(err)) - })?; - context.validate_blinded(&disseminated).map_err(|err| { - warn!(?err, "Disseminated envelope failed the decision bindings"); + Error::SpecificError(SpecificError::DataTooLarge(format!( + "Envelope too large for dissemination: {provided} > {max}" + ))) + }, + )?, + }; + self.signature_collector + .broadcast_dissemination( + validator_pubkey, + cluster.committee_id(), + dissemination, + ) + .map_err(|err| { + warn!(?err, "Envelope dissemination broadcast failed"); record_outcome(metrics::ENVELOPE_OUTCOME_FAILED); - Error::SpecificError(err) + Error::SpecificError(SpecificError::DisseminationBroadcastFailed(err)) })?; - disseminated - }; - // Sign before the publish gate so every operator contributes its share and the - // builder can reconstruct the threshold signature. `payload_root` is trusted from - // the builder operator by design (SIP-94 §6). let epoch = slot.epoch(E::slots_per_epoch()); let domain_hash = self.get_domain(epoch, Domain::BeaconBuilder); - let signing_root = signed_blinded.signing_root(domain_hash); + let signing_root = local_blinded.signing_root(domain_hash); let remaining = deadline.saturating_duration_since(Instant::now()); let signature = Self::collect_within( remaining, @@ -4100,25 +4203,6 @@ impl + 'static> ValidatorStore record_outcome(metrics::ENVELOPE_OUTCOME_FAILED); })?; - // Publish gate: the caller publishes every `Ok`, so only the builder operator may - // return one. - if !context.built_locally { - let local_root = local_blinded.tree_hash_root(); - let disseminated_root = signed_blinded.tree_hash_root(); - info!( - ?local_root, - ?disseminated_root, - "Signed another operator's envelope, skipping publish (expected)" - ); - record_outcome(metrics::ENVELOPE_OUTCOME_NOT_BUILT_LOCALLY); - return Err(Error::SpecificError( - SpecificError::EnvelopeNotBuiltLocally { - local_root, - disseminated_root, - }, - )); - } - record_outcome(metrics::ENVELOPE_OUTCOME_PUBLISHED); Ok(SignedExecutionPayloadEnvelope { message: envelope, diff --git a/anchor/validator_store/src/testing/envelope_signing.rs b/anchor/validator_store/src/testing/envelope_signing.rs index 66642cbf6..869f737f1 100644 --- a/anchor/validator_store/src/testing/envelope_signing.rs +++ b/anchor/validator_store/src/testing/envelope_signing.rs @@ -1,22 +1,29 @@ //! Envelope-signing duty tests (SIP-94 §6, disseminate-and-sign). -use std::sync::LazyLock; +use std::{sync::LazyLock, time::Duration}; use bls::{FixedBytesExtended, PublicKeyBytes}; use eth2::types::FullBlockContents; use signature_collector::CollectionError; use slashing_protection::Safe; use ssv_types::{ - OperatorId, consensus::BlindedExecutionPayloadEnvelope, dissemination::EnvelopeDissemination, - msgid::Role, partial_sig::PartialSignatureKind, + OperatorId, + consensus::{ + BEACON_ROLE_PROPOSER, BlindedExecutionPayloadEnvelope, DataVersion, ProposerConsensusData, + ValidatorDuty, + }, + dissemination::EnvelopeDissemination, + msgid::Role, + partial_sig::PartialSignatureKind, }; use ssz::Encode; use ssz_types::VariableList; use tree_hash::TreeHash; use types::{ - BeaconBlock, BeaconBlockGloas, Domain, EmptyBlock, EthSpec, ExecutionPayloadEnvelope, - ExecutionPayloadGloas, ExecutionRequestsGloas, Hash256, MainnetEthSpec, - SignedExecutionPayloadEnvelope, SignedRoot, Slot, consts::gloas::BUILDER_INDEX_SELF_BUILD, + BeaconBlock, BeaconBlockGloas, ChainSpec, Domain, EmptyBlock, EthSpec, + ExecutionPayloadEnvelope, ExecutionPayloadGloas, ExecutionRequestsGloas, ForkName, Hash256, + MainnetEthSpec, SignedExecutionPayloadEnvelope, SignedRoot, Slot, + consts::gloas::BUILDER_INDEX_SELF_BUILD, }; use validator_store::{UnsignedBlock, ValidatorStore}; @@ -154,6 +161,111 @@ async fn sign_envelope( .await } +/// Runs the body of the detached non-builder task for the harness validator at `TEST_SLOT`, +/// awaiting it directly: the same code `sign_block` spawns, minus the executor. +async fn run_non_builder_task( + harness: &ValidatorStoreTestHarness, + pubkey: PublicKeyBytes, + context: DecidedBlockContext, +) -> Result<(), Error> { + let (validator, cluster) = harness.validator_store.get_validator_and_cluster(pubkey)?; + harness + .validator_store + .clone() + .sign_disseminated_envelope(validator, cluster, context, Slot::new(TEST_SLOT)) + .await +} + +/// Number of captured signature collections of the envelope kind. +fn envelope_collection_count(harness: &ValidatorStoreTestHarness) -> usize { + harness + .captured_calls + .lock() + .iter() + .filter(|call| call.metadata.kind == PartialSignatureKind::Envelope) + .count() +} + +/// Waits for the detached non-builder task to reach signature collection and returns the root +/// it signed. Bounded so a task that never signs fails the test instead of hanging it. +async fn wait_for_envelope_signing_root(harness: &ValidatorStoreTestHarness) -> Hash256 { + tokio::time::timeout(Duration::from_secs(5), async { + loop { + let signed_root = harness + .captured_calls + .lock() + .iter() + .find(|call| call.metadata.kind == PartialSignatureKind::Envelope) + .map(|call| call.signing_root); + if let Some(root) = signed_root { + return root; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("the non-builder task must reach signature collection") +} + +/// Gives spawned tasks a chance to run before a negative assertion: a wrongly spawned +/// non-builder task with a valid dissemination already stored would sign immediately. +async fn let_spawned_tasks_run() { + for _ in 0..8 { + tokio::task::yield_now().await; + } + tokio::time::sleep(Duration::from_millis(50)).await; +} + +/// A Gloas block at `TEST_SLOT` whose bid names `builder_index` and commits to the default +/// execution requests, so `self_build_envelope(block.canonical_root())` binds to it. +fn gloas_block_with_bid(spec: &ChainSpec, builder_index: u64) -> BeaconBlock { + // `EmptyBlock::empty` fixes the slot to `spec.genesis_slot`, so the slot is set on the + // inner struct before wrapping. + let mut gloas_block = BeaconBlockGloas::::empty(spec); + gloas_block.slot = Slot::new(TEST_SLOT); + let bid = &mut gloas_block.body.signed_execution_payload_bid.message; + bid.builder_index = builder_index; + bid.execution_requests_root = + ExecutionRequestsGloas::::default().tree_hash_root(); + BeaconBlock::Gloas(gloas_block) +} + +/// The envelope for `block` that every decision binding accepts. +fn envelope_for_block( + block: &BeaconBlock, +) -> ExecutionPayloadEnvelope { + let mut envelope = self_build_envelope(block.canonical_root()); + envelope.parent_beacon_block_root = block.parent_root(); + envelope +} + +/// The consensus value a fixed-decision mock returns so `sign_block` decides `block` for the +/// harness validator regardless of the local proposal. +fn decided_consensus_data( + committee: &CommitteeSetup, + pubkey: PublicKeyBytes, + block: &BeaconBlock, +) -> ProposerConsensusData { + let validator_index = committee.validators[0] + .index + .expect("the harness validator has a beacon index"); + ProposerConsensusData { + duty: ValidatorDuty { + r#type: BEACON_ROLE_PROPOSER, + pub_key: pubkey, + slot: Slot::new(TEST_SLOT), + validator_index, + committee_index: 0, + committee_length: 0, + committees_at_slot: 0, + validator_committee_index: 0, + validator_sync_committee_indices: Default::default(), + }, + version: DataVersion::from(ForkName::Gloas), + data_ssz: VariableList::new(block.as_ssz_bytes()).expect("block bytes should fit"), + } +} + /// Before-values of the three envelope outcome labels, taken under `METRIC_TEST_LOCK`. struct OutcomeCounters { published: crate::metrics::IntCounter, @@ -406,44 +518,66 @@ async fn builder_with_binding_mismatch_broadcasts_nothing() { // ==================== Non-builder path ==================== -/// A non-builder signs the disseminated envelope's root and returns the sentinel error so the -/// caller never publishes. -#[tokio::test(flavor = "multi_thread")] -async fn non_builder_signs_disseminated_root_and_returns_sentinel() { +/// Lighthouse's envelope callback on a non-builder returns the delegated sentinel at once: the +/// share is signed by the task `sign_block` spawned, and the callback must neither wait for the +/// dissemination nor collect a second signature. +#[tokio::test(start_paused = true)] +async fn non_builder_callback_returns_delegated_sentinel_without_signing() { let _guard = METRIC_TEST_LOCK.lock().await; let (harness, pubkey) = gloas_harness(); - // The local envelope differs from the disseminated one in its payload content. let local_envelope = self_build_envelope(test_decided_root()); let mut builder_envelope = local_envelope.clone(); builder_envelope.payload.block_number = 42; seed_context(&harness, pubkey, context_for(&builder_envelope, false)); - let disseminated = insert_dissemination(&harness, pubkey, &builder_envelope); + insert_dissemination(&harness, pubkey, &builder_envelope); let counters = OutcomeCounters::snapshot(); - let domain_hash = envelope_domain_hash(&harness); - let result = sign_envelope(&harness, pubkey, local_envelope).await; assert!( matches!( result, Err(Error::SpecificError( - SpecificError::EnvelopeNotBuiltLocally { .. } + SpecificError::EnvelopeNonBuilderDelegated { .. } )) ), - "a non-builder must return the sentinel after contributing, got {result:?}" + "a non-builder callback must return the delegated sentinel, got {result:?}" ); + counters.assert_deltas(0, 0, 0); + assert_no_outward_action(&harness, "from a non-builder callback"); +} + +/// The non-builder task signs the disseminated envelope's root, never its own, and broadcasts +/// no dissemination. +#[tokio::test(flavor = "multi_thread")] +async fn non_builder_task_signs_disseminated_root() { + let _guard = METRIC_TEST_LOCK.lock().await; + let (harness, pubkey) = gloas_harness(); + // The builder's envelope differs from what this operator's own node built. + let mut builder_envelope = self_build_envelope(test_decided_root()); + builder_envelope.payload.block_number = 42; + let context = context_for(&builder_envelope, false); + seed_context(&harness, pubkey, context); + let disseminated = insert_dissemination(&harness, pubkey, &builder_envelope); + let counters = OutcomeCounters::snapshot(); + + let domain_hash = envelope_domain_hash(&harness); + + run_non_builder_task(&harness, pubkey, context) + .await + .expect("the non-builder task must contribute its share"); + counters.assert_deltas(0, 1, 0); let captured = harness.captured_calls.lock(); assert_eq!( captured.len(), 1, - "the non-builder must still contribute its signature share" + "exactly one signature share is contributed" ); assert_eq!( captured[0].signing_root, disseminated.signing_root(domain_hash), - "the non-builder must sign the disseminated envelope's root, not its own" + "the non-builder must sign the disseminated envelope's root" ); assert!( harness.captured_disseminations.lock().is_empty(), @@ -451,16 +585,18 @@ async fn non_builder_signs_disseminated_root_and_returns_sentinel() { ); } -/// Without a dissemination, the non-builder times out at the deadline having signed nothing. +/// Without a dissemination, the non-builder task times out at the deadline having signed +/// nothing. #[tokio::test(start_paused = true)] async fn non_builder_without_dissemination_times_out() { let _guard = METRIC_TEST_LOCK.lock().await; let (harness, pubkey) = gloas_harness(); let envelope = self_build_envelope(test_decided_root()); - seed_context(&harness, pubkey, context_for(&envelope, false)); + let context = context_for(&envelope, false); + seed_context(&harness, pubkey, context); let counters = OutcomeCounters::snapshot(); - let result = sign_envelope(&harness, pubkey, envelope).await; + let result = run_non_builder_task(&harness, pubkey, context).await; assert!( matches!( @@ -481,14 +617,15 @@ async fn non_builder_rejects_binding_mismatched_dissemination() { let _guard = METRIC_TEST_LOCK.lock().await; let (harness, pubkey) = gloas_harness(); let local_envelope = self_build_envelope(test_decided_root()); - seed_context(&harness, pubkey, context_for(&local_envelope, false)); + let context = context_for(&local_envelope, false); + seed_context(&harness, pubkey, context); // Disseminated envelope binds to a different beacon block root. let mut forged = local_envelope.clone(); forged.beacon_block_root = Hash256::repeat_byte(0xDD); insert_dissemination(&harness, pubkey, &forged); let counters = OutcomeCounters::snapshot(); - let result = sign_envelope(&harness, pubkey, local_envelope).await; + let result = run_non_builder_task(&harness, pubkey, context).await; assert!( matches!( @@ -511,7 +648,8 @@ async fn non_builder_rejects_undecodable_dissemination() { let _guard = METRIC_TEST_LOCK.lock().await; let (harness, pubkey) = gloas_harness(); let envelope = self_build_envelope(test_decided_root()); - seed_context(&harness, pubkey, context_for(&envelope, false)); + let context = context_for(&envelope, false); + seed_context(&harness, pubkey, context); harness.dissemination_store.insert( pubkey, EnvelopeDissemination { @@ -521,7 +659,7 @@ async fn non_builder_rejects_undecodable_dissemination() { ); let counters = OutcomeCounters::snapshot(); - let result = sign_envelope(&harness, pubkey, envelope).await; + let result = run_non_builder_task(&harness, pubkey, context).await; assert!( matches!( @@ -812,3 +950,139 @@ async fn sign_block_then_matching_envelope_succeeds() { "the builder provenance recorded by sign_block must drive a dissemination" ); } + +/// Mixed bid, the case Lighthouse's callback cannot serve: this operator's own node took an +/// external builder's bid, the cluster decided another operator's self-build block. `sign_block` +/// must spawn the non-builder task, which signs the disseminated envelope's root once it arrives. +/// A later callback for the slot returns the delegated sentinel and adds no second share. +#[tokio::test(flavor = "multi_thread")] +async fn sign_block_with_another_operators_self_build_block_signs_its_envelope() { + let _guard = METRIC_TEST_LOCK.lock().await; + let (committee, pubkey) = single_validator_committee(); + let spec = gloas_at_genesis_spec(); + let decided_block = gloas_block_with_bid(&spec, BUILDER_INDEX_SELF_BUILD); + let decided = decided_consensus_data(&committee, pubkey, &decided_block); + let harness = ValidatorStoreTestHarness::new_with_options( + vec![committee], + OperatorId(1), + HarnessOptions { + decider: MockConsensusDecider::fixed_after_barrier(&decided, 1), + ..gloas_options() + }, + ); + let local_block = gloas_block_with_bid(&spec, 7); + assert_ne!( + local_block.canonical_root(), + decided_block.canonical_root(), + "the fixture must model a decided block this operator did not build" + ); + let builder_envelope = envelope_for_block(&decided_block); + let domain_hash = envelope_domain_hash(&harness); + + harness + .validator_store + .sign_block( + pubkey, + UnsignedBlock::Full(FullBlockContents::Block(local_block)), + Slot::new(TEST_SLOT), + ) + .await + .expect("the Gloas block duty must sign successfully"); + // The builder operator's dissemination arrives after the block decided. + let disseminated = insert_dissemination(&harness, pubkey, &builder_envelope); + + let signed_root = wait_for_envelope_signing_root(&harness).await; + + assert_eq!( + signed_root, + disseminated.signing_root(domain_hash), + "the spawned task must sign the disseminated envelope's root" + ); + assert!( + harness.captured_disseminations.lock().is_empty(), + "a non-builder must never broadcast a dissemination" + ); + + // Lighthouse's callback for the same slot: nothing to publish, no second share. + let result = sign_envelope(&harness, pubkey, envelope_for_block(&decided_block)).await; + assert!( + matches!( + result, + Err(Error::SpecificError( + SpecificError::EnvelopeNonBuilderDelegated { .. } + )) + ), + "the callback on a non-builder must return the delegated sentinel, got {result:?}" + ); + assert_eq!( + envelope_collection_count(&harness), + 1, + "the callback must not collect a second envelope share" + ); +} + +/// The builder operator signs from Lighthouse's callback only: `sign_block` on a block this +/// operator built spawns no non-builder task. A valid dissemination is stored up front so a +/// wrongly spawned task would sign immediately and be caught. +#[tokio::test(flavor = "multi_thread")] +async fn sign_block_as_builder_spawns_no_non_builder_task() { + let _guard = METRIC_TEST_LOCK.lock().await; + let (harness, pubkey) = gloas_harness(); + let block = gloas_block_with_bid(&harness.spec, BUILDER_INDEX_SELF_BUILD); + insert_dissemination(&harness, pubkey, &envelope_for_block(&block)); + + harness + .validator_store + .sign_block( + pubkey, + UnsignedBlock::Full(FullBlockContents::Block(block)), + Slot::new(TEST_SLOT), + ) + .await + .expect("the Gloas block duty must sign successfully"); + let_spawned_tasks_run().await; + + assert_eq!( + envelope_collection_count(&harness), + 0, + "the builder must not sign its envelope before Lighthouse's callback" + ); +} + +/// A decided block that committed to an external builder's bid has no self-build envelope duty, +/// so `sign_block` spawns nothing even though this operator did not build the block. +#[tokio::test(flavor = "multi_thread")] +async fn sign_block_with_external_build_decision_spawns_no_non_builder_task() { + let _guard = METRIC_TEST_LOCK.lock().await; + let (committee, pubkey) = single_validator_committee(); + let spec = gloas_at_genesis_spec(); + let decided_block = gloas_block_with_bid(&spec, 7); + let decided = decided_consensus_data(&committee, pubkey, &decided_block); + let harness = ValidatorStoreTestHarness::new_with_options( + vec![committee], + OperatorId(1), + HarnessOptions { + decider: MockConsensusDecider::fixed_after_barrier(&decided, 1), + ..gloas_options() + }, + ); + let local_block = gloas_block_with_bid(&spec, BUILDER_INDEX_SELF_BUILD); + insert_dissemination(&harness, pubkey, &envelope_for_block(&decided_block)); + + harness + .validator_store + .sign_block( + pubkey, + UnsignedBlock::Full(FullBlockContents::Block(local_block)), + Slot::new(TEST_SLOT), + ) + .await + .expect("the Gloas block duty must sign successfully"); + let_spawned_tasks_run().await; + + assert_eq!( + envelope_collection_count(&harness), + 0, + "an external-build decision carries no self-build envelope duty" + ); +} From a64a8844d5988a30d8e21d0e3b5e1340c0e51299 Mon Sep 17 00:00:00 2001 From: shane-moore Date: Wed, 2 Sep 2026 18:02:48 -0700 Subject: [PATCH 2/2] test(validator_store): pin the non-builder spawn placement Lighthouse can call sign_block twice for one slot: a second block-service notification for the same slot hit three of four operators at one devnet slot, about 12 s after the first call. The non-builder envelope task is spawned only after sign_abstract_block, so the repeat is rejected by slashing protection as SameData before it can spawn a second task. Nothing pinned that placement: the existing spawn-path tests disable slashing protection and call sign_block once. Add a slashing-enabled test that calls sign_block twice and expects one envelope collection; it fails with two collections if the spawn is moved above the slashing check. Also blind the local envelope only on the builder path. The non-builder callback now returns before using it, so hashing the full payload up front was wasted work on every non-builder duty. --- anchor/validator_store/src/lib.rs | 4 +- .../src/testing/envelope_signing.rs | 68 +++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/anchor/validator_store/src/lib.rs b/anchor/validator_store/src/lib.rs index 5f39af2b1..2d813a76a 100644 --- a/anchor/validator_store/src/lib.rs +++ b/anchor/validator_store/src/lib.rs @@ -4093,7 +4093,6 @@ impl + 'static> ValidatorStore } let context = self.get_decided_block_context(validator_pubkey, slot)?; - let local_blinded = BlindedExecutionPayloadEnvelope::from_full(&envelope); let record_outcome = |outcome: &str| { metrics::inc_counter_vec(&metrics::ENVELOPE_SIGNING_OUTCOMES, &[outcome]); @@ -4151,6 +4150,9 @@ impl + 'static> ValidatorStore }, )); } + // Blind here rather than up front: hashing the full payload is the costly step, and + // only this builder path uses the result. + let local_blinded = BlindedExecutionPayloadEnvelope::from_full(&envelope); context.validate_blinded(&local_blinded).map_err(|err| { warn!(?err, "Local envelope failed the decision bindings"); record_outcome(metrics::ENVELOPE_OUTCOME_FAILED); diff --git a/anchor/validator_store/src/testing/envelope_signing.rs b/anchor/validator_store/src/testing/envelope_signing.rs index 869f737f1..2ad9ec514 100644 --- a/anchor/validator_store/src/testing/envelope_signing.rs +++ b/anchor/validator_store/src/testing/envelope_signing.rs @@ -1086,3 +1086,71 @@ async fn sign_block_with_external_build_decision_spawns_no_non_builder_task() { "an external-build decision carries no self-build envelope duty" ); } + +/// Lighthouse can invoke `sign_block` twice for one slot: a second block-service notification +/// for the same slot was observed on a devnet. The repeat must fail slashing protection inside +/// `sign_abstract_block` as `SameData` before `sign_block` reaches the non-builder spawn, so only +/// the first call's task signs. This pins the spawn's placement after `sign_abstract_block`: the +/// other spawn-path tests disable slashing protection and call `sign_block` once, so a spawn +/// moved above the slashing check would pass them and double-sign on the devnet. +#[tokio::test(flavor = "multi_thread")] +async fn repeated_sign_block_for_the_same_slot_spawns_one_non_builder_task() { + let _guard = METRIC_TEST_LOCK.lock().await; + let (committee, pubkey) = single_validator_committee(); + let spec = gloas_at_genesis_spec(); + let decided_block = gloas_block_with_bid(&spec, BUILDER_INDEX_SELF_BUILD); + let decided = decided_consensus_data(&committee, pubkey, &decided_block); + let harness = ValidatorStoreTestHarness::new_with_options( + vec![committee], + OperatorId(1), + HarnessOptions { + decider: MockConsensusDecider::fixed_after_barrier(&decided, 1), + disable_slashing_protection: false, + ..gloas_options() + }, + ); + let local_block = gloas_block_with_bid(&spec, 7); + assert_ne!( + local_block.canonical_root(), + decided_block.canonical_root(), + "the fixture must model a decided block this operator did not build" + ); + // Stored up front so the legitimately spawned task signs immediately, and a wrongly spawned + // second task would too. + insert_dissemination(&harness, pubkey, &envelope_for_block(&decided_block)); + + harness + .validator_store + .sign_block( + pubkey, + UnsignedBlock::Full(FullBlockContents::Block(local_block.clone())), + Slot::new(TEST_SLOT), + ) + .await + .expect("the first Gloas block duty must sign successfully"); + wait_for_envelope_signing_root(&harness).await; + + let repeat = harness + .validator_store + .sign_block( + pubkey, + UnsignedBlock::Full(FullBlockContents::Block(local_block)), + Slot::new(TEST_SLOT), + ) + .await; + assert!( + matches!(repeat, Err(Error::SameData)), + "a repeated sign_block for the same slot must be rejected by slashing protection as SameData, got {repeat:?}" + ); + let_spawned_tasks_run().await; + + assert_eq!( + envelope_collection_count(&harness), + 1, + "a repeated sign_block must not spawn a second non-builder task" + ); + assert!( + harness.captured_disseminations.lock().is_empty(), + "a non-builder must never broadcast a dissemination" + ); +}