From 2c121da5e285655a44646fb581e711e90ba346e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Mon, 26 Oct 2026 18:08:56 -0300 Subject: [PATCH 01/16] feat(validator): fetch v2 proposer duties and submit proposer preferences Signed proposer preferences must carry the shuffling dependent root, which only v2 of the proposer duties endpoint reports (v1's root is one epoch later). Add both calls to the beacon node trait, with the fallback sending preferences to every node since each one gossips what it accepts. --- crates/validator/src/beacon_node/fallback.rs | 68 ++++++++++++++++ crates/validator/src/beacon_node/http.rs | 85 ++++++++++++++++++++ crates/validator/src/beacon_node/mock.rs | 73 +++++++++++++++++ crates/validator/src/beacon_node/mod.rs | 22 +++++ 4 files changed, 248 insertions(+) diff --git a/crates/validator/src/beacon_node/fallback.rs b/crates/validator/src/beacon_node/fallback.rs index 04b4652e4..933ea34b7 100644 --- a/crates/validator/src/beacon_node/fallback.rs +++ b/crates/validator/src/beacon_node/fallback.rs @@ -256,6 +256,11 @@ impl BeaconNodeApi for FallbackBeaconNode { .await } + async fn proposer_duties_v2(&self, epoch: Epoch) -> Result { + self.try_each("proposer_duties_v2", |node| node.proposer_duties_v2(epoch)) + .await + } + async fn attestation_data(&self, slot: Slot, fork: ForkName) -> Result { self.try_each("attestation_data", |node| node.attestation_data(slot, fork)) .await @@ -342,6 +347,26 @@ impl BeaconNodeApi for FallbackBeaconNode { .await } + /// To every node, since each one gossips what it accepts: a preference + /// only the first node heard reaches the mesh through that node alone, and + /// builders subscribed elsewhere would ignore the slot. + async fn submit_proposer_preferences( + &self, + fork: ForkName, + preferences: &[gloas::SignedProposerPreferences], + ) -> Result<()> { + self.try_all( + "submit_proposer_preferences", + "it will not gossip these proposer preferences", + Some( + "no node took this client's proposer preferences; builders' bids for those \ + slots are ignored, and from gloas a node may not know the fee recipient", + ), + |node| node.submit_proposer_preferences(fork, preferences), + ) + .await + } + /// Fails over like any query: the envelope the node cached is only on the /// node that produced the block, and one that did not answers 404. async fn execution_payload_envelope( @@ -1061,6 +1086,49 @@ mod tests { .expect_err("the record is gone"); } + #[tokio::test] + async fn proposer_preferences_reach_every_node_past_a_failing_one() { + use ethlambda_types::beacon::containers::gloas::{ + ProposerPreferences, SignedProposerPreferences, + }; + + let preference = SignedProposerPreferences { + message: ProposerPreferences { + proposal_slot: 70, + ..Default::default() + }, + ..Default::default() + }; + let fallback = FallbackBeaconNode::new(vec![ + MockBeaconNode::new(), + MockBeaconNode::failing("middle down"), + MockBeaconNode::new(), + ]); + + fallback + .submit_proposer_preferences(ForkName::Fulu, std::slice::from_ref(&preference)) + .await + .expect("two of three accepted"); + + for position in [0, 2] { + assert_eq!( + *fallback.nodes[position] + .submitted_preferences + .lock() + .expect("lock"), + vec![(ForkName::Fulu, vec![preference.clone()])], + "node {position}" + ); + } + assert!( + fallback.nodes[1] + .submitted_preferences + .lock() + .expect("lock") + .is_empty() + ); + } + #[tokio::test] async fn registrations_reach_every_node() { use ethlambda_types::beacon::builder::{ diff --git a/crates/validator/src/beacon_node/http.rs b/crates/validator/src/beacon_node/http.rs index 599119890..f127acf38 100644 --- a/crates/validator/src/beacon_node/http.rs +++ b/crates/validator/src/beacon_node/http.rs @@ -627,6 +627,18 @@ impl BeaconNodeApi for HttpBeaconNode { }) } + /// [`Self::proposer_duties`] against the v2 path, whose `dependent_root` + /// is the shuffling dependent root proposer preferences must carry. + async fn proposer_duties_v2(&self, epoch: Epoch) -> Result { + let response: DutiesResponse> = self + .get(&format!("/eth/v2/validator/duties/proposer/{epoch}")) + .await?; + Ok(ProposerDuties { + dependent_root: parse_root(&response.dependent_root)?, + duties: response.data, + }) + } + async fn attestation_data(&self, slot: Slot, fork: ForkName) -> Result { let response: DataResponse = self.get(&Self::attestation_data_path(slot, fork)).await?; @@ -807,6 +819,24 @@ impl BeaconNodeApi for HttpBeaconNode { .await } + /// JSON, which every beacon node must accept, an empty list never reaching + /// the wire. The gloas container serialises as the endpoint's JSON itself. + async fn submit_proposer_preferences( + &self, + fork: ForkName, + preferences: &[gloas::SignedProposerPreferences], + ) -> Result<()> { + if preferences.is_empty() { + return Ok(()); + } + self.post_no_content( + "/eth/v1/validator/proposer_preferences", + &preferences, + Some(fork.as_str()), + ) + .await + } + async fn execution_payload_envelope( &self, slot: Slot, @@ -1469,6 +1499,61 @@ mod tests { assert!(served.seen.lock().expect("lock").is_empty()); } + #[tokio::test] + async fn proposer_preferences_are_posted_as_json_with_the_fork_header() { + use ethlambda_types::beacon::primitives::{BlsSignature, H160}; + + let preferences = vec![gloas::SignedProposerPreferences { + message: gloas::ProposerPreferences { + dependent_root: Root::from([9u8; 32]), + proposal_slot: 70, + validator_index: 4, + fee_recipient: H160([0xcd; 20]), + target_gas_limit: 36_000_000, + }, + signature: BlsSignature([5; 96]), + }]; + let served = serve(200, vec![], vec![]).await; + served + .node + .submit_proposer_preferences(ForkName::Fulu, &preferences) + .await + .expect("accepted"); + + let seen = served.seen.lock().expect("lock"); + assert_eq!(seen.len(), 1); + assert!( + seen[0] + .0 + .ends_with("/eth/v1/validator/proposer_preferences") + ); + assert_eq!(seen[0].2["eth-consensus-version"], "fulu"); + let decoded: Vec = + serde_json::from_slice(&seen[0].1).expect("the body is the endpoint's JSON"); + assert_eq!(decoded, preferences); + } + + #[tokio::test] + async fn an_empty_preference_list_never_reaches_the_wire() { + let served = serve(200, vec![], vec![]).await; + served + .node + .submit_proposer_preferences(ForkName::Fulu, &[]) + .await + .expect("a no-op"); + assert!(served.seen.lock().expect("lock").is_empty()); + } + + #[tokio::test] + async fn v2_proposer_duties_use_the_v2_path() { + let reply = br#"{"dependent_root":"0x0909090909090909090909090909090909090909090909090909090909090909","execution_optimistic":false,"data":[]}"#.to_vec(); + let served = serve(200, vec![("content-type", "application/json")], reply).await; + let duties = served.node.proposer_duties_v2(3).await.expect("decodes"); + assert_eq!(duties.dependent_root, Root::from([9u8; 32])); + let seen = served.seen.lock().expect("lock"); + assert!(seen[0].0.ends_with("/eth/v2/validator/duties/proposer/3")); + } + #[tokio::test] async fn a_blinded_publish_maps_each_status() { for (status, expected) in [ diff --git a/crates/validator/src/beacon_node/mock.rs b/crates/validator/src/beacon_node/mock.rs index 409e3aedf..c56287708 100644 --- a/crates/validator/src/beacon_node/mock.rs +++ b/crates/validator/src/beacon_node/mock.rs @@ -64,6 +64,11 @@ pub struct MockBeaconNode { pub validators: Vec, pub duties: Mutex>, pub proposers: Mutex>, + /// What `proposer_duties_v2` answers with, by epoch, like `proposers`. + pub proposers_v2: Mutex>, + /// Every proposer preferences batch `submit_proposer_preferences` + /// received, with the fork it was sent under, in call order. + pub submitted_preferences: Mutex)>>, pub attestation_data: Option, /// The slot and proposer the mock will produce a block for. `None` makes /// `produce_block` fail, the way an unset `attestation_data` does. @@ -136,6 +141,7 @@ pub struct MockBeaconNode { pub duties_calls: Mutex, pub validator_indices_calls: Mutex, pub proposer_duties_calls: Mutex, + pub proposer_duties_v2_calls: Mutex, pub attestation_data_calls: Mutex, /// Sync duties by the epoch they are asked at. An epoch never set answers /// no duties, so tests that do not care about sync committees are @@ -253,6 +259,44 @@ impl MockBeaconNode { } } + /// Set the v2 proposer duties for `epoch`, replacing any earlier ones: a + /// reorg is the same epoch answering with another dependent root. + pub fn set_proposers_v2( + &self, + epoch: Epoch, + dependent_root: Root, + duties: Vec, + ) { + let mut stored = self.proposers_v2.lock().expect("lock"); + let entry = ProposerDuties { + dependent_root, + duties, + }; + match stored + .iter_mut() + .find(|(stored_epoch, _)| *stored_epoch == epoch) + { + Some((_, existing)) => *existing = entry, + None => stored.push((epoch, entry)), + } + } + + pub fn with_proposers_v2( + self, + epoch: Epoch, + dependent_root: Root, + duties: Vec, + ) -> Self { + self.set_proposers_v2(epoch, dependent_root, duties); + self + } + + /// How many times `proposer_duties_v2` has been called, successes and + /// failures alike. + pub fn proposer_duties_v2_call_count(&self) -> usize { + *self.proposer_duties_v2_calls.lock().expect("lock") + } + pub fn with_attestation_data(mut self, slot: Slot) -> Self { self.attestation_data = Some(AttestationData { slot, @@ -634,6 +678,22 @@ impl BeaconNodeApi for MockBeaconNode { }) } + async fn proposer_duties_v2(&self, epoch: Epoch) -> Result { + self.guard("proposer_duties_v2")?; + *self.proposer_duties_v2_calls.lock().expect("lock") += 1; + self.proposers_v2 + .lock() + .expect("lock") + .iter() + .find(|(stored, _)| *stored == epoch) + .map(|(_, duties)| duties.clone()) + .ok_or_else(|| Error::BeaconNode { + url: "mock".to_string(), + failure: BeaconNodeFailure::Request, + detail: format!("no v2 proposer duties for epoch {epoch}"), + }) + } + async fn attestation_data(&self, slot: Slot, fork: ForkName) -> Result { self.guard("attestation_data")?; *self.attestation_data_calls.lock().expect("lock") += 1; @@ -777,6 +837,19 @@ impl BeaconNodeApi for MockBeaconNode { Ok(()) } + async fn submit_proposer_preferences( + &self, + fork: ForkName, + preferences: &[gloas::SignedProposerPreferences], + ) -> Result<()> { + self.guard("submit_proposer_preferences")?; + self.submitted_preferences + .lock() + .expect("lock") + .push((fork, preferences.to_vec())); + Ok(()) + } + async fn execution_payload_envelope( &self, slot: Slot, diff --git a/crates/validator/src/beacon_node/mod.rs b/crates/validator/src/beacon_node/mod.rs index ed292c5f4..0f6ac0ab7 100644 --- a/crates/validator/src/beacon_node/mod.rs +++ b/crates/validator/src/beacon_node/mod.rs @@ -322,6 +322,13 @@ pub trait BeaconNodeApi: Send + Sync { /// treats the two the same way will act on a stale proposer schedule. async fn proposer_duties(&self, epoch: Epoch) -> Result; + /// Every proposer duty in `epoch` with v2's `dependent_root`: the block at + /// `compute_start_slot_at_epoch(epoch - 1) - 1`, the shuffling dependent + /// root fulu's proposer lookahead fixes the epoch's proposers from. Signed + /// proposer preferences must carry exactly this root, which v1's + /// definition (one epoch later) is not. + async fn proposer_duties_v2(&self, epoch: Epoch) -> Result; + /// Produce the attestation data for `slot`, which is in `fork`. /// /// No committee index is ever a parameter here. Before gloas the query @@ -416,6 +423,21 @@ pub trait BeaconNodeApi: Send + Sync { registrations: &[SignedValidatorRegistrationV1], ) -> Result<()>; + /// Submit signed proposer preferences, which the node validates with the + /// `proposer_preferences` gossip rules and publishes. `fork` is the + /// `Eth-Consensus-Version`: the fork of the current epoch. + /// + /// A 400 naming per-index failures is an `Err`: the whole call failed for + /// the caller's purposes. The caller re-submits the entries next slot, and + /// the node answers per entry, so the ones that already went out are + /// deduplicated by the node. State on the node's gossip, so failover sends + /// it to every node. + async fn submit_proposer_preferences( + &self, + fork: ForkName, + preferences: &[gloas::SignedProposerPreferences], + ) -> Result<()>; + /// The envelope the node cached for the self-built payload of the block /// whose root is `block_root`, proposed at `slot`. /// From ecba6abbcc730a5989241ae59b88a1f666aa38ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Mon, 26 Oct 2026 18:10:11 -0300 Subject: [PATCH 02/16] feat(validator): sign proposer preferences under the gossip rule's domain The gossip validator verifies a preference at the fork version of the epoch before its proposal, while the validator guide signs with the head state. The two differ only in the first epoch of a fork, where the gossip rule is what peers accept, so sign for that one. --- crates/validator/src/signing.rs | 116 +++++++++++++++++++++++++++++++- 1 file changed, 113 insertions(+), 3 deletions(-) diff --git a/crates/validator/src/signing.rs b/crates/validator/src/signing.rs index cafa4099a..d43a36029 100644 --- a/crates/validator/src/signing.rs +++ b/crates/validator/src/signing.rs @@ -9,14 +9,15 @@ use ethlambda_types::beacon::builder::ValidatorRegistrationV1; use ethlambda_types::beacon::config::Config; use ethlambda_types::beacon::constants::{ DOMAIN_AGGREGATE_AND_PROOF, DOMAIN_BEACON_ATTESTER, DOMAIN_BEACON_BUILDER, - DOMAIN_BEACON_PROPOSER, DOMAIN_CONTRIBUTION_AND_PROOF, DOMAIN_PTC_ATTESTER, DOMAIN_RANDAO, - DOMAIN_SELECTION_PROOF, DOMAIN_SYNC_COMMITTEE, DOMAIN_SYNC_COMMITTEE_SELECTION_PROOF, + DOMAIN_BEACON_PROPOSER, DOMAIN_CONTRIBUTION_AND_PROOF, DOMAIN_PROPOSER_PREFERENCES, + DOMAIN_PTC_ATTESTER, DOMAIN_RANDAO, DOMAIN_SELECTION_PROOF, DOMAIN_SYNC_COMMITTEE, + DOMAIN_SYNC_COMMITTEE_SELECTION_PROOF, }; use ethlambda_types::beacon::containers::altair::{ ContributionAndProof, SyncAggregatorSelectionData, }; use ethlambda_types::beacon::containers::gloas::{ - ExecutionPayloadEnvelope, PayloadAttestationData, + ExecutionPayloadEnvelope, PayloadAttestationData, ProposerPreferences, }; use ethlambda_types::beacon::containers::shared::AttestationData; use ethlambda_types::beacon::primitives::{ @@ -25,6 +26,7 @@ use ethlambda_types::beacon::primitives::{ }; // `AttestationData`, `Checkpoint`, `Fork` and `SigningData` all live in // `containers::shared`, verified against `containers/shared.rs:158,120,97,220`. +use ethlambda_types::beacon::preset; use ethlambda_types::beacon::signing::{ compute_builder_domain, compute_domain, compute_epoch_at_slot, compute_signing_root, }; @@ -316,6 +318,31 @@ impl SigningContext { self.sign_root(store, ®istration.pubkey, root) } + /// A proposer preferences message's signature, under + /// `DOMAIN_PROPOSER_PREFERENCES` at the fork version of the epoch before + /// the proposal's. + /// + /// That is the domain the `proposer_preferences` gossip rule verifies + /// against: `get_domain(state, .., proposal_epoch)` on the dependent + /// block's state advanced to the lookahead start, whose fork is the one + /// in force an epoch before the proposal. The validator guide signs with + /// the head state instead; the two differ only in the first epoch of a + /// fork, where this one is the one peers accept. Not slashable. + pub fn sign_proposer_preferences( + &self, + store: &ValidatorStore, + pubkey: &BlsPubkey, + preferences: &ProposerPreferences, + ) -> Result { + let proposal_epoch = compute_epoch_at_slot(preferences.proposal_slot); + let domain = self.domain( + DOMAIN_PROPOSER_PREFERENCES, + proposal_epoch.saturating_sub(preset::MIN_SEED_LOOKAHEAD), + ); + let root = compute_signing_root(preferences.hash_tree_root(), domain); + self.sign_root(store, pubkey, root) + } + /// Sign a payload timeliness committee vote on behalf of `pubkey`. /// /// Not slashable by the specification, and deduplicated per validator and @@ -590,6 +617,89 @@ mod tests { assert!(!verify(&pubkey, &signature, envelope_root)); } + /// A config whose gloas fork starts at epoch 500000, fulu being active + /// well before it, with a non-zero validators root. + fn gloas_scheduled_context() -> (SigningContext, Epoch) { + const GLOAS_EPOCH: Epoch = 500_000; + let mut config = Config::mainnet(); + config.gloas_fork_epoch = GLOAS_EPOCH; + ( + SigningContext { + config, + genesis_validators_root: Root::repeat_byte(0x42), + }, + GLOAS_EPOCH, + ) + } + + fn preferences_for(slot: Slot) -> ProposerPreferences { + ProposerPreferences { + dependent_root: Root::repeat_byte(0x11), + proposal_slot: slot, + validator_index: 3, + fee_recipient: ethlambda_types::beacon::primitives::H160([0xab; 20]), + target_gas_limit: 36_000_000, + } + } + + /// The first gloas epoch's preference is signed at the fork of the epoch + /// before it (fulu), which is what the gossip rule verifies against, and + /// not under the gloas version the validator guide's head-state signer + /// would use. + #[test] + fn a_preference_for_the_first_gloas_epoch_is_signed_under_the_fulu_version() { + let (store, pubkey) = store_with_key(); + let (context, gloas_epoch) = gloas_scheduled_context(); + let preferences = preferences_for(gloas_epoch * preset::SLOTS_PER_EPOCH + 2); + + let signature = context + .sign_proposer_preferences(&store, &pubkey, &preferences) + .expect("signs"); + + let under = |version| { + compute_signing_root( + preferences.hash_tree_root(), + compute_domain( + DOMAIN_PROPOSER_PREFERENCES, + version, + context.genesis_validators_root, + ), + ) + }; + assert!(verify( + &pubkey, + &signature, + under(context.config.fulu_fork_version) + )); + assert!(!verify( + &pubkey, + &signature, + under(context.config.gloas_fork_version) + )); + } + + /// One epoch later the epoch before the proposal's is itself gloas. + #[test] + fn a_preference_for_the_second_gloas_epoch_is_signed_under_the_gloas_version() { + let (store, pubkey) = store_with_key(); + let (context, gloas_epoch) = gloas_scheduled_context(); + let preferences = preferences_for((gloas_epoch + 1) * preset::SLOTS_PER_EPOCH); + + let signature = context + .sign_proposer_preferences(&store, &pubkey, &preferences) + .expect("signs"); + + let root = compute_signing_root( + preferences.hash_tree_root(), + compute_domain( + DOMAIN_PROPOSER_PREFERENCES, + context.config.gloas_fork_version, + context.genesis_validators_root, + ), + ); + assert!(verify(&pubkey, &signature, root)); + } + #[test] fn a_randao_reveal_verifies_under_the_randao_signing_root() { let (store, pubkey) = store_with_key(); From 6b09d6ab2896b5849c43e9bb506d7d8589fa2ec6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Mon, 26 Oct 2026 18:12:16 -0300 Subject: [PATCH 03/16] feat(validator): publish proposer preferences for gloas proposals From gloas a proposer's fee recipient and gas limit travel in signed proposer preferences, and a node ignores trustless bids for a slot whose proposer published none. A per-slot service signs one per owned proposal in the current and next gloas epoch, keyed by (slot, dependent root) so a reorg re-publishes, and sends it to every node. --- crates/validator/src/lib.rs | 1 + crates/validator/src/metrics.rs | 31 ++ crates/validator/src/preferences.rs | 610 ++++++++++++++++++++++++++++ 3 files changed, 642 insertions(+) create mode 100644 crates/validator/src/preferences.rs diff --git a/crates/validator/src/lib.rs b/crates/validator/src/lib.rs index 7e2732809..9deedf9e3 100644 --- a/crates/validator/src/lib.rs +++ b/crates/validator/src/lib.rs @@ -40,6 +40,7 @@ pub mod http_api; pub mod keys; pub mod metrics; pub mod payload_attestation; +pub mod preferences; pub mod proposal; pub mod proposal_guard; pub mod proposer_settings; diff --git a/crates/validator/src/metrics.rs b/crates/validator/src/metrics.rs index 64d7ca0cc..92fc44b2f 100644 --- a/crates/validator/src/metrics.rs +++ b/crates/validator/src/metrics.rs @@ -134,6 +134,27 @@ static BUILDER_REGISTRATION_FAILURES_TOTAL: LazyLock = LazyLock::new .unwrap() }); +/// Proposer preferences a beacon node accepted. Counted per preference, so a +/// full epoch's worth reads as that many. +static PROPOSER_PREFERENCES_PUBLISHED_TOTAL: LazyLock = LazyLock::new(|| { + register_int_counter!( + "ethlambda_validator_proposer_preferences_published_total", + "Proposer preferences a beacon node accepted" + ) + .unwrap() +}); + +/// Proposer preference batches no beacon node accepted. The next slot retries, +/// so this is a warning sign: until one is accepted, builders' bids for the +/// slot are ignored. +static PROPOSER_PREFERENCES_FAILURES_TOTAL: LazyLock = LazyLock::new(|| { + register_int_counter!( + "ethlambda_validator_proposer_preferences_failures_total", + "Proposer preference batches no beacon node accepted" + ) + .unwrap() +}); + /// Blinded blocks a beacon node handed to its builder. static BLINDED_BLOCKS_PROPOSED_TOTAL: LazyLock = LazyLock::new(|| { register_int_counter!( @@ -358,6 +379,8 @@ pub fn init() { LazyLock::force(&BLOCKS_BROADCAST_NOT_IMPORTED_TOTAL); LazyLock::force(&BUILDER_REGISTRATIONS_SENT_TOTAL); LazyLock::force(&BUILDER_REGISTRATION_FAILURES_TOTAL); + LazyLock::force(&PROPOSER_PREFERENCES_PUBLISHED_TOTAL); + LazyLock::force(&PROPOSER_PREFERENCES_FAILURES_TOTAL); LazyLock::force(&BLINDED_BLOCKS_PROPOSED_TOTAL); LazyLock::force(&BLINDED_BLOCKS_UNCONFIRMED_TOTAL); LazyLock::force(&BLOCKS_REFUSED_TOTAL); @@ -428,6 +451,14 @@ pub fn inc_builder_registration_failures() { BUILDER_REGISTRATION_FAILURES_TOTAL.inc(); } +pub fn inc_proposer_preferences_published(count: usize) { + PROPOSER_PREFERENCES_PUBLISHED_TOTAL.inc_by(count as u64); +} + +pub fn inc_proposer_preferences_failures() { + PROPOSER_PREFERENCES_FAILURES_TOTAL.inc(); +} + pub fn inc_blinded_blocks_proposed() { BLINDED_BLOCKS_PROPOSED_TOTAL.inc(); } diff --git a/crates/validator/src/preferences.rs b/crates/validator/src/preferences.rs new file mode 100644 index 000000000..667a88875 --- /dev/null +++ b/crates/validator/src/preferences.rs @@ -0,0 +1,610 @@ +//! Signed proposer preferences: how a validator tells builders, and from +//! gloas its own beacon node, where its proposals should pay and which gas +//! limit to target. +//! +//! # Why it exists +//! +//! From gloas, beacon-APIs makes a signed `ProposerPreferences` the source of a +//! proposer's fee recipient and gas limit, superseding `prepare_beacon_proposer`. +//! It is also the trustless-bid channel: a node ignores p2p builder bids for a +//! slot whose proposer published no preferences, so a validator that stays +//! silent self-builds every gloas proposal. +//! +//! # Why v2 proposer duties +//! +//! A preference names the `dependent_root` its duty was computed from, and the +//! gossip rule compares it with the node's own shuffling dependent root. Fulu's +//! proposer lookahead fixes an epoch's proposers one epoch earlier than v1's +//! definition says, so only v2's `dependent_root` is the one a preference must +//! carry. The duty loop keeps its own v1 schedule; this service fetches v2 for +//! itself. +//! +//! # The signing domain +//! +//! See [`SigningContext::sign_proposer_preferences`]: the gossip rule's domain, +//! at the fork of the epoch before the proposal's, which is not the validator +//! guide's head-state one in the first epoch of a fork. +//! +//! # Why every slot +//! +//! [`PreferencesService::poll`] runs each slot rather than each epoch. A reorg +//! changes a dependent root, which makes the old preference unusable and needs +//! a new one; a validator can become active, or be given a fee recipient, +//! mid-epoch; and a node that refused a send must be tried again. What was +//! already accepted is remembered per `(slot, dependent_root)`, so a quiet +//! slot costs the two duty fetches and no signing. +//! +//! # Why every node gets them +//! +//! Each node gossips what it accepts, so the fallback client sends to all of +//! them: a preference only one node heard reaches builders through that node's +//! mesh alone. +//! +//! # No replacement +//! +//! A node deduplicates on `(slot, dependent_root)`, so a preference cannot be +//! replaced for the same slot and root. A fee recipient changed after a +//! preference went out applies from the next dependent root onward only. +//! +//! # Failures are never propagated +//! +//! A missed preference costs a builder bid and, from gloas, possibly the +//! node's knowledge of the fee recipient, never a duty. A failed send is logged +//! and counted, and the next slot retries. + +use std::collections::HashSet; +use std::sync::Arc; +use std::sync::atomic::{AtomicBool, Ordering}; + +use ethlambda_types::beacon::containers::gloas::{ProposerPreferences, SignedProposerPreferences}; +use ethlambda_types::beacon::fork::ForkName; +use ethlambda_types::beacon::preset; +use ethlambda_types::beacon::primitives::{BlsPubkey, Epoch, Root, Slot}; +use ethlambda_types::beacon::signing::compute_epoch_at_slot; +use tokio::sync::RwLock; +use tracing::{debug, info, warn}; + +use crate::beacon_node::BeaconNodeApi; +use crate::beacon_node::dto::{encode_hex, parse_pubkey}; +use crate::keys::ValidatorStore; +use crate::proposer_settings::ProposerSettings; +use crate::signing::SigningContext; + +/// Clears [`PreferencesService`]'s `in_flight` flag on every exit from a poll, +/// a panic included, so one failed poll cannot stop every later one. +struct InFlightGuard<'a>(&'a AtomicBool); + +impl Drop for InFlightGuard<'_> { + fn drop(&mut self) { + self.0.store(false, Ordering::Release); + } +} + +/// Signs and publishes proposer preferences. +pub struct PreferencesService { + beacon_node: Arc, + context: Arc, + settings: Arc, + /// `(proposal_slot, dependent_root)` pairs a node accepted, so a + /// preference is signed and sent once per dependent root. A reorg that + /// moves the root sends the slot again; pruned below the current epoch. + published: std::sync::Mutex>, + /// Epochs whose missing fee recipients were already warned about, so the + /// warning is once per epoch rather than once per slot. Pruned like + /// `published`. + warned_epochs: std::sync::Mutex>, + /// One poll at a time: a slow node must not stack polls up. + in_flight: AtomicBool, +} + +impl PreferencesService { + pub fn new( + beacon_node: Arc, + context: Arc, + settings: Arc, + ) -> Self { + Self { + beacon_node, + context, + settings, + published: std::sync::Mutex::new(HashSet::new()), + warned_epochs: std::sync::Mutex::new(HashSet::new()), + in_flight: AtomicBool::new(false), + } + } + + /// Publish what this client's validators still owe for the current and the + /// next epoch, as of `current_slot`. Called once per slot, off the slot + /// loop, so a slow node cannot delay the next slot's duties. + /// + /// Returns at once when the previous poll is still running. + pub async fn poll(self: Arc, current_slot: Slot, store: Arc>) { + if self.in_flight.swap(true, Ordering::AcqRel) { + return; + } + let _guard = InFlightGuard(&self.in_flight); + + let epoch = compute_epoch_at_slot(current_slot); + // Snapshotted with the guard dropped at once: nothing below awaits a + // node while holding the store. + let pubkeys: HashSet = store.read().await.pubkeys().into_iter().collect(); + let fork = self.context.config.fork_at_epoch(epoch); + + for target in [epoch, epoch + 1] { + if self.context.config.fork_at_epoch(target) < ForkName::Gloas { + continue; + } + self.publish_epoch(fork, current_slot, target, &pubkeys, &store) + .await; + } + + let epoch_start = epoch * preset::SLOTS_PER_EPOCH; + self.published + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .retain(|(slot, _)| *slot >= epoch_start); + self.warned_epochs + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .retain(|warned| *warned >= epoch); + } + + /// One target epoch: fetch its v2 duties, sign what is still owed and send it. + async fn publish_epoch( + &self, + fork: ForkName, + current_slot: Slot, + target: Epoch, + pubkeys: &HashSet, + store: &Arc>, + ) { + let duties = match self.beacon_node.proposer_duties_v2(target).await { + Ok(duties) => duties, + Err(err) => { + debug!(epoch = target, %err, "Could not fetch proposer duties for preferences"); + return; + } + }; + + let mut owed = Vec::new(); + let mut no_fee_recipient = 0usize; + for duty in &duties.duties { + // Started slots are skipped: gossip would IGNORE them. + if duty.slot <= current_slot { + continue; + } + let Ok(pubkey) = parse_pubkey(&duty.pubkey) else { + continue; + }; + if !pubkeys.contains(&pubkey) { + continue; + } + let already = self + .published + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .contains(&(duty.slot, duties.dependent_root)); + if already { + continue; + } + let Some(fee_recipient) = self.settings.fee_recipient(&pubkey) else { + no_fee_recipient += 1; + continue; + }; + owed.push(( + pubkey, + ProposerPreferences { + dependent_root: duties.dependent_root, + proposal_slot: duty.slot, + validator_index: duty.validator_index, + fee_recipient, + target_gas_limit: self.settings.gas_limit(&pubkey), + }, + )); + } + + if no_fee_recipient > 0 { + let first_warning = self + .warned_epochs + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .insert(target); + if first_warning { + warn!( + epoch = target, + skipped = no_fee_recipient, + "Validators without a fee recipient publish no proposer preferences" + ); + } + } + if owed.is_empty() { + return; + } + + // At most a few per epoch, so signing inline is cheap; the read guard + // is released before the submit. + let mut signed = Vec::with_capacity(owed.len()); + { + let store = store.read().await; + for (pubkey, message) in owed { + match self + .context + .sign_proposer_preferences(&store, &pubkey, &message) + { + Ok(signature) => signed.push(SignedProposerPreferences { message, signature }), + Err(err) => { + warn!( + pubkey = %encode_hex(&pubkey.0), + slot = message.proposal_slot, + %err, + "Could not sign proposer preferences; skipping them" + ); + } + } + } + } + if signed.is_empty() { + return; + } + + match self + .beacon_node + .submit_proposer_preferences(fork, &signed) + .await + { + Ok(()) => { + let count = signed.len(); + self.published + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .extend( + signed.iter().map(|entry| { + (entry.message.proposal_slot, entry.message.dependent_root) + }), + ); + crate::metrics::inc_proposer_preferences_published(count); + info!(epoch = target, count, "Proposer preferences published"); + } + Err(err) => { + warn!(epoch = target, count = signed.len(), %err, "No beacon node accepted the proposer preferences"); + crate::metrics::inc_proposer_preferences_failures(); + } + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::beacon_node::dto::ProposerDutyDto; + use crate::beacon_node::mock::MockBeaconNode; + use ethlambda_types::beacon::config::Config; + use ethlambda_types::beacon::constants::DOMAIN_PROPOSER_PREFERENCES; + use ethlambda_types::beacon::primitives::{Bytes32, H160, HashTreeRoot as _}; + use ethlambda_types::beacon::signing::{compute_domain, compute_signing_root}; + + const GLOAS_EPOCH: Epoch = 500_000; + const FEE_RECIPIENT: H160 = H160([0xab; 20]); + + fn context() -> Arc { + let mut config = Config::mainnet(); + config.gloas_fork_epoch = GLOAS_EPOCH; + Arc::new(SigningContext { + config, + genesis_validators_root: Root::repeat_byte(0x42), + }) + } + + /// `count` distinct keys. The secrets are small integers, which are valid + /// BLS scalars, so no keystore is needed. + fn store_with_keys(count: usize) -> (ValidatorStore, Vec) { + let mut store = ValidatorStore::new(); + let pubkeys = (0..count) + .map(|index| { + let mut secret = [0u8; 32]; + secret[24..].copy_from_slice(&(index as u64 + 1).to_be_bytes()); + store.insert_secret("test", &secret).expect("inserts") + }) + .collect(); + (store, pubkeys) + } + + fn duty(pubkey: &BlsPubkey, validator_index: u64, slot: Slot) -> ProposerDutyDto { + ProposerDutyDto { + pubkey: encode_hex(&pubkey.0), + validator_index, + slot, + } + } + + fn settings(fee_recipient: Option) -> Arc { + Arc::new(ProposerSettings::new(Bytes32::ZERO, fee_recipient)) + } + + fn service( + node: MockBeaconNode, + settings: Arc, + ) -> (Arc>, Arc) { + let node = Arc::new(node); + ( + Arc::new(PreferencesService::new(node.clone(), context(), settings)), + node, + ) + } + + fn slot_in(epoch: Epoch, offset: Slot) -> Slot { + epoch * preset::SLOTS_PER_EPOCH + offset + } + + fn submitted(node: &MockBeaconNode) -> Vec<(ForkName, Vec)> { + node.submitted_preferences.lock().expect("lock").clone() + } + + fn verifies(pubkey: &BlsPubkey, entry: &SignedProposerPreferences) -> bool { + use blst::min_pk::{PublicKey, Signature}; + + let context = context(); + let proposal_epoch = compute_epoch_at_slot(entry.message.proposal_slot); + let domain = context.domain(DOMAIN_PROPOSER_PREFERENCES, proposal_epoch - 1); + assert_eq!( + domain, + compute_domain( + DOMAIN_PROPOSER_PREFERENCES, + context + .config + .fork_version(context.config.fork_at_epoch(proposal_epoch - 1)), + context.genesis_validators_root, + ) + ); + let root = compute_signing_root(entry.message.hash_tree_root(), domain); + let pk = PublicKey::from_bytes(&pubkey.0).expect("valid pubkey"); + let sig = Signature::from_bytes(&entry.signature.0).expect("valid signature"); + sig.verify( + true, + root.as_slice(), + b"BLS_SIG_BLS12381G2_XMD:SHA-256_SSWU_RO_POP_", + &[], + &pk, + true, + ) == blst::BLST_ERROR::BLST_SUCCESS + } + + #[tokio::test] + async fn publishes_for_this_clients_gloas_duties_in_the_current_and_next_epoch() { + let (store, pubkeys) = store_with_keys(1); + let store = Arc::new(RwLock::new(store)); + let settings = settings(Some(FEE_RECIPIENT)); + settings.set_gas_limit(&pubkeys[0], 31_000_000); + let current = slot_in(GLOAS_EPOCH, 1); + let node = MockBeaconNode::new() + .with_proposers_v2( + GLOAS_EPOCH, + Root::repeat_byte(0x11), + vec![duty(&pubkeys[0], 3, slot_in(GLOAS_EPOCH, 10))], + ) + .with_proposers_v2( + GLOAS_EPOCH + 1, + Root::repeat_byte(0x22), + vec![duty(&pubkeys[0], 3, slot_in(GLOAS_EPOCH + 1, 4))], + ); + let (service, node) = service(node, settings); + + service.poll(current, store).await; + + let sent = submitted(&node); + assert_eq!(sent.len(), 2, "one send per target epoch"); + let mut entries: Vec<_> = sent + .iter() + .inspect(|(fork, _)| assert_eq!(*fork, ForkName::Gloas)) + .flat_map(|(_, entries)| entries.clone()) + .collect(); + entries.sort_by_key(|entry| entry.message.proposal_slot); + assert_eq!(entries.len(), 2); + let expected = [ + (slot_in(GLOAS_EPOCH, 10), Root::repeat_byte(0x11)), + (slot_in(GLOAS_EPOCH + 1, 4), Root::repeat_byte(0x22)), + ]; + for (entry, (slot, root)) in entries.iter().zip(expected) { + assert_eq!(entry.message.proposal_slot, slot); + assert_eq!(entry.message.dependent_root, root); + assert_eq!(entry.message.validator_index, 3); + assert_eq!(entry.message.fee_recipient, FEE_RECIPIENT); + assert_eq!(entry.message.target_gas_limit, 31_000_000); + assert!(verifies(&pubkeys[0], entry), "slot {slot}"); + } + } + + #[tokio::test] + async fn a_second_poll_does_not_resubmit() { + let (store, pubkeys) = store_with_keys(1); + let store = Arc::new(RwLock::new(store)); + let node = MockBeaconNode::new().with_proposers_v2( + GLOAS_EPOCH, + Root::repeat_byte(0x11), + vec![duty(&pubkeys[0], 3, slot_in(GLOAS_EPOCH, 10))], + ); + let (service, node) = service(node, settings(Some(FEE_RECIPIENT))); + let current = slot_in(GLOAS_EPOCH, 1); + + service.clone().poll(current, store.clone()).await; + service.poll(current + 1, store).await; + + assert_eq!(submitted(&node).len(), 1); + } + + #[tokio::test] + async fn a_changed_dependent_root_resubmits() { + let (store, pubkeys) = store_with_keys(1); + let store = Arc::new(RwLock::new(store)); + let slot = slot_in(GLOAS_EPOCH, 10); + let node = MockBeaconNode::new().with_proposers_v2( + GLOAS_EPOCH, + Root::repeat_byte(0x11), + vec![duty(&pubkeys[0], 3, slot)], + ); + let (service, node) = service(node, settings(Some(FEE_RECIPIENT))); + let current = slot_in(GLOAS_EPOCH, 1); + + service.clone().poll(current, store.clone()).await; + node.set_proposers_v2( + GLOAS_EPOCH, + Root::repeat_byte(0x33), + vec![duty(&pubkeys[0], 3, slot)], + ); + service.poll(current + 1, store).await; + + let sent = submitted(&node); + assert_eq!(sent.len(), 2); + assert_eq!(sent[1].1[0].message.dependent_root, Root::repeat_byte(0x33)); + assert!(verifies(&pubkeys[0], &sent[1].1[0])); + } + + #[tokio::test] + async fn pre_gloas_epochs_are_skipped() { + let (store, pubkeys) = store_with_keys(1); + let store = Arc::new(RwLock::new(store)); + let node = MockBeaconNode::new().with_proposers_v2( + GLOAS_EPOCH, + Root::repeat_byte(0x11), + vec![duty(&pubkeys[0], 3, slot_in(GLOAS_EPOCH, 10))], + ); + let (service, node) = service(node, settings(Some(FEE_RECIPIENT))); + + // The epoch before gloas: only the next, gloas, epoch is owed. + service + .clone() + .poll(slot_in(GLOAS_EPOCH - 1, 1), store.clone()) + .await; + let sent = submitted(&node); + assert_eq!(sent.len(), 1); + assert_eq!(sent[0].0, ForkName::Fulu, "the current epoch's fork"); + assert_eq!(sent[0].1[0].message.proposal_slot, slot_in(GLOAS_EPOCH, 10)); + assert_eq!(node.proposer_duties_v2_call_count(), 1); + + // Two epochs before: nothing is submitted and no v2 call is made. + let (service, node) = service_for_two_epochs_before(&pubkeys); + service.poll(slot_in(GLOAS_EPOCH - 2, 1), store).await; + assert!(submitted(&node).is_empty()); + assert_eq!(node.proposer_duties_v2_call_count(), 0); + } + + fn service_for_two_epochs_before( + pubkeys: &[BlsPubkey], + ) -> (Arc>, Arc) { + let node = MockBeaconNode::new().with_proposers_v2( + GLOAS_EPOCH - 1, + Root::repeat_byte(0x11), + vec![duty(&pubkeys[0], 3, slot_in(GLOAS_EPOCH - 1, 10))], + ); + service(node, settings(Some(FEE_RECIPIENT))) + } + + #[tokio::test] + async fn started_slots_and_other_validators_are_skipped() { + let (store, pubkeys) = store_with_keys(2); + // The store holds only the first key; the second is another client's. + let (held, _) = store_with_keys(1); + let held = Arc::new(RwLock::new(held)); + let current = slot_in(GLOAS_EPOCH, 5); + let node = MockBeaconNode::new().with_proposers_v2( + GLOAS_EPOCH, + Root::repeat_byte(0x11), + vec![ + duty(&pubkeys[0], 3, current), + duty(&pubkeys[0], 3, current - 1), + duty(&pubkeys[1], 4, slot_in(GLOAS_EPOCH, 10)), + duty(&pubkeys[0], 3, slot_in(GLOAS_EPOCH, 12)), + ], + ); + drop(store); + let (service, node) = service(node, settings(Some(FEE_RECIPIENT))); + + service.poll(current, held).await; + + let sent = submitted(&node); + assert_eq!(sent.len(), 1); + assert_eq!(sent[0].1.len(), 1); + assert_eq!(sent[0].1[0].message.proposal_slot, slot_in(GLOAS_EPOCH, 12)); + assert_eq!(sent[0].1[0].message.validator_index, 3); + } + + #[tokio::test] + async fn a_validator_without_a_fee_recipient_is_skipped() { + let (store, pubkeys) = store_with_keys(2); + let store = Arc::new(RwLock::new(store)); + let settings = settings(None); + settings.set_fee_recipient(&pubkeys[1], FEE_RECIPIENT); + let node = MockBeaconNode::new().with_proposers_v2( + GLOAS_EPOCH, + Root::repeat_byte(0x11), + vec![ + duty(&pubkeys[0], 3, slot_in(GLOAS_EPOCH, 10)), + duty(&pubkeys[1], 4, slot_in(GLOAS_EPOCH, 11)), + ], + ); + let (service, node) = service(node, settings); + + service.poll(slot_in(GLOAS_EPOCH, 1), store).await; + + let sent = submitted(&node); + assert_eq!(sent.len(), 1); + assert_eq!(sent[0].1.len(), 1); + assert_eq!(sent[0].1[0].message.validator_index, 4); + } + + #[tokio::test] + async fn a_failed_submit_is_retried_next_poll() { + let (store, pubkeys) = store_with_keys(1); + let store = Arc::new(RwLock::new(store)); + let node = MockBeaconNode::new().with_proposers_v2( + GLOAS_EPOCH, + Root::repeat_byte(0x11), + vec![duty(&pubkeys[0], 3, slot_in(GLOAS_EPOCH, 10))], + ); + node.failures + .lock() + .expect("lock") + .insert("submit_proposer_preferences", "node refused".to_string()); + let (service, node) = service(node, settings(Some(FEE_RECIPIENT))); + let current = slot_in(GLOAS_EPOCH, 1); + + service.clone().poll(current, store.clone()).await; + assert!(submitted(&node).is_empty()); + + node.failures.lock().expect("lock").clear(); + service.poll(current + 1, store).await; + assert_eq!(submitted(&node).len(), 1, "sent once the node recovered"); + } + + #[tokio::test] + async fn overlapping_polls_do_not_run_twice() { + let (store, pubkeys) = store_with_keys(1); + let store = Arc::new(RwLock::new(store)); + let node = MockBeaconNode::new().with_proposers_v2( + GLOAS_EPOCH, + Root::repeat_byte(0x11), + vec![duty(&pubkeys[0], 3, slot_in(GLOAS_EPOCH, 10))], + ); + let (service, node) = service(node, settings(Some(FEE_RECIPIENT))); + + service.in_flight.store(true, Ordering::Release); + service + .clone() + .poll(slot_in(GLOAS_EPOCH, 1), store.clone()) + .await; + assert!(submitted(&node).is_empty()); + assert_eq!(node.proposer_duties_v2_call_count(), 0); + assert!( + service.in_flight.load(Ordering::Acquire), + "the running poll's flag is not cleared by the one that bounced" + ); + + service.in_flight.store(false, Ordering::Release); + service.clone().poll(slot_in(GLOAS_EPOCH, 1), store).await; + assert_eq!(submitted(&node).len(), 1); + assert!( + !service.in_flight.load(Ordering::Acquire), + "cleared after a poll" + ); + } +} From c5fbbf37af81caebdc8b97fd801a2166ebf259f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Mon, 26 Oct 2026 18:14:15 -0300 Subject: [PATCH 04/16] feat(validator): publish proposer preferences every slot and weigh gloas bids by the boost factor Wire the preferences service into the slot loop, unconditionally since from gloas they carry every proposal's fee recipient. produceBlockV4 now sends the configured builder_boost_factor in its body, so with --builder-proposals a pooled p2p bid can win on value; without it the factor stays 0 as before. --- crates/validator/src/beacon_node/http.rs | 57 +++++++++++++++++++----- crates/validator/src/beacon_node/mod.rs | 5 ++- crates/validator/src/lib.rs | 13 ++++++ 3 files changed, 61 insertions(+), 14 deletions(-) diff --git a/crates/validator/src/beacon_node/http.rs b/crates/validator/src/beacon_node/http.rs index f127acf38..453248c80 100644 --- a/crates/validator/src/beacon_node/http.rs +++ b/crates/validator/src/beacon_node/http.rs @@ -79,10 +79,17 @@ struct SszResponse { body: Vec, } -/// The body `produceBlockV4` requires. Self-build only, so no minimum bid, no -/// boost and no builders: the node decodes it, as the specification demands, -/// and builds locally. -const BUILDER_CONFIG: &str = r#"{"min_bid":"0","builder_boost_factor":"0","builders":[]}"#; +/// The body `produceBlockV4` requires: the node decodes it, as the +/// specification demands, even when it builds locally. +/// +/// `min_bid` stays `0` and `builders` stays empty, since the builder API +/// (relay or `builder_pubkeys` bids) is a later phase. The factor is the +/// configured `builder_boost_factor` with `--builder-proposals`, so a pooled +/// p2p bid can win on value, and `0` without it: self-build only, as before. +fn builder_config(boost_factor: Option) -> String { + let factor = boost_factor.unwrap_or(0); + format!(r#"{{"min_bid":"0","builder_boost_factor":"{factor}","builders":[]}}"#) +} /// A [`BeaconNodeApi`] backed by one beacon node's standard REST Beacon API. pub struct HttpBeaconNode { @@ -256,7 +263,7 @@ impl HttpBeaconNode { async fn post_json_for_ssz( &self, path: &str, - json_body: &'static str, + json_body: String, consensus_version: &str, ) -> Result { let url = format!("{}{path}", self.base_url); @@ -393,7 +400,11 @@ impl HttpBeaconNode { encode_hex(&request.graffiti.0), ); let response = self - .post_json_for_ssz(&path, BUILDER_CONFIG, request.fork.as_str()) + .post_json_for_ssz( + &path, + builder_config(request.builder_boost_factor), + request.fork.as_str(), + ) .await?; if response.fork != ForkName::Gloas { return Err(Error::InconsistentResponse(format!( @@ -1330,13 +1341,35 @@ mod tests { ); } - /// The body `produceBlockV4` requires: self-build only, so no builders. + /// The body `produceBlockV4` requires: no minimum bid and no builders, + /// the boost factor being the configured one or `0`. #[test] - fn the_builder_config_is_the_documented_self_build_body() { - let value: serde_json::Value = serde_json::from_str(BUILDER_CONFIG).expect("valid json"); - assert_eq!(value["min_bid"], "0"); - assert_eq!(value["builder_boost_factor"], "0"); - assert_eq!(value["builders"], serde_json::json!([])); + fn the_builder_config_carries_the_factor_or_zero() { + for (factor, expected) in [(None, "0"), (Some(250), "250"), (Some(0), "0")] { + let value: serde_json::Value = + serde_json::from_str(&builder_config(factor)).expect("valid json"); + assert_eq!(value["min_bid"], "0"); + assert_eq!(value["builder_boost_factor"], expected, "{factor:?}"); + assert_eq!(value["builders"], serde_json::json!([])); + } + } + + /// The body reaches the wire: a gloas production request posts it. + #[tokio::test] + async fn a_gloas_production_request_posts_the_factor_in_its_body() { + for (factor, expected) in [(None, "0"), (Some(250), "250")] { + let served = serve(500, vec![], vec![]).await; + let request = BlockRequest { + fork: ForkName::Gloas, + ..block_request(factor) + }; + let _ = served.node.produce_block(&request).await; + let seen = served.seen.lock().expect("lock"); + assert_eq!(seen.len(), 1); + assert!(seen[0].0.contains("/eth/v4/validator/blocks/5")); + let body: serde_json::Value = serde_json::from_slice(&seen[0].1).expect("json body"); + assert_eq!(body["builder_boost_factor"], expected); + } } /// A Beacon API double that answers every route the same way: whatever diff --git a/crates/validator/src/beacon_node/mod.rs b/crates/validator/src/beacon_node/mod.rs index 0f6ac0ab7..5c8c189de 100644 --- a/crates/validator/src/beacon_node/mod.rs +++ b/crates/validator/src/beacon_node/mod.rs @@ -133,8 +133,9 @@ pub struct BlockRequest { pub graffiti: Bytes32, /// The `builder_boost_factor` to send, or `None` when builder proposals are /// off. `None` sends `0` and refuses a blinded answer; `Some` sends the - /// factor and accepts one. Ignored from gloas, where `produceBlockV4` takes - /// the builder configuration in its body. + /// factor and accepts one. From gloas the factor travels in + /// `produceBlockV4`'s body instead of the query, so a pooled p2p bid can + /// win on value; a gloas answer is never blinded. pub builder_boost_factor: Option, } diff --git a/crates/validator/src/lib.rs b/crates/validator/src/lib.rs index 9deedf9e3..d95df82d5 100644 --- a/crates/validator/src/lib.rs +++ b/crates/validator/src/lib.rs @@ -70,6 +70,7 @@ use crate::beacon_node::{BeaconNodeApi, Published}; use crate::duties::DutiesService; use crate::keys::ValidatorStore; use crate::payload_attestation::PayloadAttestationService; +use crate::preferences::PreferencesService; use crate::proposal::ProposalService; use crate::proposer_settings::ProposerSettings; use crate::registration::RegistrationService; @@ -236,6 +237,14 @@ pub async fn run(config: ValidatorConfig) -> Result<()> { )) }); + // Unconditional, as Lighthouse does: from gloas the preferences carry the + // fee recipient of every proposal, builders or not. See the module doc. + let preferences = Arc::new(PreferencesService::new( + beacon_node.clone(), + context.clone(), + settings.clone(), + )); + if let Some(address) = config.keymanager { let token = http_api::load_or_create_token(&config.validators_dir)?; let context = http_api::KeymanagerContext { @@ -382,6 +391,10 @@ pub async fn run(config: ValidatorConfig) -> Result<()> { if let Some(registration) = ®istration { tokio::spawn(registration.clone().register_changed(epoch, store.clone())); } + + // Likewise off the slot loop: the poll is single-flight, and a slow + // node must not delay the next slot's duties. + tokio::spawn(preferences.clone().poll(slot, store.clone())); } } From efaa12057e52400edfa994dd7b3d2708929f27f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Mon, 26 Oct 2026 18:15:45 -0300 Subject: [PATCH 05/16] test(rpc): the validator client's proposer preferences pass this node's gossip rules The signing domain is the part of the preferences flow most likely to drift between the signer and the verifier, so drive the validator client's own signer and HTTP client against the real route and gossip rules. --- .../rpc/src/beacon/builder_market_tests.rs | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/crates/net/rpc/src/beacon/builder_market_tests.rs b/crates/net/rpc/src/beacon/builder_market_tests.rs index 4f89fd600..981a0177e 100644 --- a/crates/net/rpc/src/beacon/builder_market_tests.rs +++ b/crates/net/rpc/src/beacon/builder_market_tests.rs @@ -1060,6 +1060,74 @@ async fn valid_preferences_are_cached_and_published_once() { assert_eq!(world.network.proposer_preferences.lock().unwrap().len(), 1); } +/// The validator client's own signer and HTTP client against this node's +/// gossip rules: the domain `sign_proposer_preferences` picks (the fork of the +/// epoch before the proposal's) is the one `validate_proposer_preferences_gossip` +/// verifies under, and the JSON the client posts is what the route decodes. +/// +/// The fork-boundary case (a preference for the first gloas epoch, signed +/// under the fulu version) is not driven here: this world's schedule has gloas +/// at epoch 0 and a second world in the epoch before the fork would be a new +/// chain fixture. `signing.rs` pins that domain in a unit test. +#[tokio::test] +async fn preferences_signed_by_the_validator_client_pass_this_nodes_gossip_rules() { + use ethlambda_validator::{ + beacon_node::{BeaconNodeApi as _, http::HttpBeaconNode}, + keys::ValidatorStore, + signing::SigningContext, + }; + + let world = World::new(); + let proposer = world.prepared().proposer; + let dependent_root = dependent_root_at(&world.state, world.head_root, SLOT) + .expect("the head's chain gives the slot a dependent root"); + let message = ProposerPreferences { + dependent_root, + proposal_slot: SLOT, + validator_index: proposer, + fee_recipient: address(0xaa), + target_gas_limit: 30_000_000, + }; + + let mut keys = ValidatorStore::new(); + let pubkey = keys + .insert_secret("test", &secret_key_for(proposer as usize).to_bytes()) + .expect("the test secret is a valid key"); + assert_eq!( + pubkey, + world.state.validator(proposer).unwrap().pubkey, + "the test registry's key for the proposer" + ); + let context = SigningContext { + config: chain_config(), + genesis_validators_root: world.state.genesis_validators_root(), + }; + let signature = context + .sign_proposer_preferences(&keys, &pubkey, &message) + .expect("signs"); + let signed = SignedProposerPreferences { message, signature }; + + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let address = listener.local_addr().unwrap(); + let router = world.app(None); + tokio::spawn(async move { axum::serve(listener, router).await }); + let client = HttpBeaconNode::new(format!("http://{address}")).unwrap(); + + client + .submit_proposer_preferences( + ethlambda_types::beacon::fork::ForkName::Gloas, + std::slice::from_ref(&signed), + ) + .await + .expect("the node accepts what the validator client signed"); + + assert_eq!( + *world.network.proposer_preferences.lock().unwrap(), + vec![signed.clone()] + ); + assert_eq!(world.market.preferences(SLOT, dependent_root), Some(signed)); +} + #[tokio::test] async fn cached_preferences_resubmitted_are_a_200_without_a_publish() { // Needs only the market: an identical entry short-circuits the rules. From 8e3fd4ec0d6f9dae02313417d3f3fd0e6ecd3ff3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Mon, 26 Oct 2026 18:17:04 -0300 Subject: [PATCH 06/16] docs: describe the validator client's proposer preferences Document the per-slot preferences, their two counters, the signing domain departure from the validator guide, and that the boost factor now also weighs gloas p2p bids. --- CLAUDE.md | 7 +++++-- docs/cli.md | 15 ++++++++++++++- docs/metrics.md | 2 ++ docs/spec_deviations.md | 23 ++++++++++++++++++++++- 4 files changed, 43 insertions(+), 4 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index dec9283c7..0b14398a0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -864,8 +864,11 @@ transitions are in `ethlambda-types`, per the section above. Nothing above `SlotClock` picks the offsets by each slot's own fork, and `/eth/v1/config/spec` supplies them. With `--builder-proposals`, pre-gloas slots register validators with the node's builder - and may sign a blinded block, published to the producing node only. It - still keeps no slashing-protection record; PTC votes have an in-memory + and may sign a blinded block, published to the producing node only. From the + epoch before gloas it signs proposer preferences every slot for its proposals in + the current and next epoch (`PreferencesService`, v2 duties, the gossip rule's + signing domain), unconditionally, and `produceBlockV4` carries the boost factor. + It still keeps no slashing-protection record; PTC votes have an in-memory `(validator, slot)` dedup only, and envelopes are unguarded. `get_head_node` is one bottom-up walk for every fork, with the current slot's fork selecting the payload rules and the boost gate, so a diff --git a/docs/cli.md b/docs/cli.md index 998f1e8fd..f6c480485 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -291,7 +291,7 @@ implementation's, so none of the [common flags](#common-flags) apply to it. | `--graffiti` | empty | Text for the graffiti field of proposed blocks, for every validator the keymanager API gives no graffiti of its own. At most 32 bytes as UTF-8, right-padded with zeros. Refused rather than truncated if longer. An ethlambda beacon node appends both clients' versions to it (see [`rpc.md`](rpc.md#validator-endpoints)) | | `--enable-keymanager` | off | Serve the keymanager API: keystores, plus each validator's own fee recipient, graffiti and gas limit. Off by default because it mutates key material | | `--builder-proposals` | off | Propose builder blocks: register each validator with the beacon node's builder (an ethlambda node: `--builder`) and accept a blinded block when the builder's bid wins. Fulu and earlier only; from gloas the builder market works through proposer preferences instead | -| `--builder-boost-factor` | `100` | How a builder's bid is weighed against the local payload: the local payload wins iff `local >= factor * bid / 100`. `0` prefers the local payload unless it cannot be built. Requires `--builder-proposals`; conflicts with `--prefer-builder-proposals` | +| `--builder-boost-factor` | `100` | How a builder's bid is weighed against the local payload: the local payload wins iff `local >= factor * bid / 100`. `0` prefers the local payload unless it cannot be built. From gloas it is also sent in `produceBlockV4`'s body, so a pooled p2p bid can win on value; without `--builder-proposals` the body carries `0`. Requires `--builder-proposals`; conflicts with `--prefer-builder-proposals` | | `--prefer-builder-proposals` | off | Take the builder's block whenever there is a valid bid, unless the execution client asks to override the builder (sends `builder_boost_factor = u64::MAX`). Requires `--builder-proposals`; conflicts with `--builder-boost-factor` | ### What it does today @@ -317,6 +317,19 @@ produced it, with no failover. A publish that node does not confirm is logged at error and counted as unconfirmed, not as a missed proposal, since the relay may still publish the block. +From the epoch before gloas on, it also signs a proposer preferences message +(fee recipient and target gas limit) for each of its proposal slots in the +current and next gloas epoch, and sends them to every node. This happens every +slot, whether or not `--builder-proposals` is set: from gloas the preferences +are what carry a proposal's fee recipient, and a node ignores builders' bids +for a slot with none. It reads those duties from the v2 proposer duties +endpoint, whose dependent root the preferences must carry, and publishes each +`(slot, dependent root)` once, again after a reorg changes the root. A +validator with no fee recipient is skipped, with one warning per epoch naming +the count. A preference cannot be replaced for the same slot and root, so a +fee recipient changed through the keymanager API applies from the next +dependent root. + Each slot it wakes at the boundary. If one of its validators proposes that slot, it signs the RANDAO reveal, asks the beacon node for a block, checks that the block is for the slot and proposer it asked about, signs it and publishes diff --git a/docs/metrics.md b/docs/metrics.md index 1b63a76be..6439c15ea 100644 --- a/docs/metrics.md +++ b/docs/metrics.md @@ -743,6 +743,8 @@ and absent is not the same as zero. | `ethlambda_validator_fee_recipient_mismatches_total` | Counter | Blocks paying execution rewards to an address this client did not request. Should read zero forever; a non-zero value means every proposal is paying somewhere else | | `ethlambda_validator_builder_registrations_sent_total` | Counter | Builder registrations a beacon node accepted (`--builder-proposals`) | | `ethlambda_validator_builder_registration_failures_total` | Counter | Registration batches no beacon node accepted. A missed registration costs at most a builder block, never a duty | +| `ethlambda_validator_proposer_preferences_published_total` | Counter | Signed proposer preferences a beacon node accepted, counted per preference. Published for every proposal from the epoch before gloas, whether or not `--builder-proposals` is set | +| `ethlambda_validator_proposer_preferences_failures_total` | Counter | Proposer preference batches no beacon node accepted. The next slot retries; until one is accepted, builders' bids for those slots are ignored | | `ethlambda_validator_blinded_blocks_proposed_total` | Counter | Blinded blocks a beacon node handed to its builder. A subset of `blocks_proposed_total` | | `ethlambda_validator_blinded_blocks_unconfirmed_total` | Counter | Blinded blocks the beacon node did not confirm (a 5xx or no answer), which the relay may still have published. Not retried, since a second block for the slot would be an equivocation | | `ethlambda_validator_block_publication_delay_seconds` | Histogram | Slot start to block accepted. Its buckets are tighter than the attestation histogram's, because a block is due at the slot boundary rather than a third of the way in | diff --git a/docs/spec_deviations.md b/docs/spec_deviations.md index 84a27a23d..15fd6ec82 100644 --- a/docs/spec_deviations.md +++ b/docs/spec_deviations.md @@ -706,6 +706,27 @@ the beacon node's builder and may sign a blinded block (builder-specs and relay may still publish the block. - **Registrations stop at gloas.** The client sends no `register_validator` from the gloas fork epoch on: the builder market works through proposer - preferences there, and `produceBlockV4` keeps its self-build configuration. + preferences there (see the next section), and `produceBlockV4` sends the + configured boost factor with no minimum bid and no builders. Validators with no fee recipient are not registered either (the field is required), and only validators the beacon node resolved are signed for. + +## The validator client signs proposer preferences under the gossip rule's domain + +`validator.md` ("Broadcasting `SignedProposerPreferences`") signs with +`get_domain(state, DOMAIN_PROPOSER_PREFERENCES, compute_epoch_at_slot(proposal_slot))` +on the head state. `p2p-interface.md`'s `validate_proposer_preferences_gossip` +verifies against the dependent block's state advanced to the lookahead start, +so the fork version it uses is the one in force an epoch before the proposal's. + +- **ethlambda:** the validator client signs at the epoch before the proposal's + (`SigningContext::sign_proposer_preferences`), which is the domain the gossip + rule verifies. +- **Why:** the two agree except in the first epoch of a fork, where the head + state still carries the old fork version and a proposal's epoch resolves to + the new one, so the guide's signature would fail the gossip rule that peers + apply. This node's own `proposer_preferences` endpoint accepts both. +- **Published unconditionally,** not gated on `--builder-proposals`, since from + gloas the preferences carry every proposal's fee recipient. Validators with no + fee recipient are skipped (the field is required), and a preference cannot be + replaced for the same slot and dependent root. From d5db0cf081fc4598d1fd339478e91f602953fced Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Mon, 26 Oct 2026 18:35:08 -0300 Subject: [PATCH 07/16] fix(validator): send proposer preferences one per request and tighten their reporting A node answers 400 for a whole call when any entry fails, and one it holds for the same slot and root with other content is such a failure, so a batch kept every entry unpublished and re-sent until that slot passed (a restart after a fee recipient change triggers it). Send each preference alone and remember each success. Also: a failed v2 duties fetch no longer warns "duty likely missed" twice a slot (the service warns once per epoch and counts it), the fork header is the target epoch's, the poll is spawned ahead of the proposal, the signing-domain explanation is corrected (get_domain reads state.fork, so the guide and the gossip rule differ only for a proposal in a fork's first epoch signed during it), v2 dependent slot reuses the state transition's definition, and the end-to-end test now reads its duty and dependent root through the client. --- bin/ethlambda/src/validator.rs | 7 +- .../rpc/src/beacon/builder_market_tests.rs | 60 ++-- crates/net/rpc/src/beacon/validator.rs | 14 +- crates/validator/src/beacon_node/fallback.rs | 40 ++- crates/validator/src/beacon_node/mock.rs | 27 +- crates/validator/src/beacon_node/mod.rs | 13 +- crates/validator/src/lib.rs | 20 +- crates/validator/src/preferences.rs | 271 ++++++++++++------ crates/validator/src/registration.rs | 21 +- crates/validator/src/signing.rs | 15 +- crates/validator/src/test_support.rs | 27 ++ docs/cli.md | 2 +- docs/spec_deviations.md | 11 +- 13 files changed, 353 insertions(+), 175 deletions(-) create mode 100644 crates/validator/src/test_support.rs diff --git a/bin/ethlambda/src/validator.rs b/bin/ethlambda/src/validator.rs index 51b27a15a..4febabe66 100644 --- a/bin/ethlambda/src/validator.rs +++ b/bin/ethlambda/src/validator.rs @@ -81,9 +81,10 @@ pub(crate) struct ValidatorOptions { /// Propose builder blocks: register each validator with the beacon node's /// builder and accept a blinded block when the builder's bid wins. Requires - /// a beacon node started with a builder (ethlambda: `--builder`). Fulu and - /// earlier only; from gloas the builder market works through proposer - /// preferences instead. + /// a beacon node started with a builder (ethlambda: `--builder`). The + /// registrations and blinded blocks are fulu and earlier only; from gloas + /// the builder market works through proposer preferences, and this flag's + /// boost factor weighs p2p bids in `produceBlockV4`. #[arg(long, default_value = "false")] pub(crate) builder_proposals: bool, diff --git a/crates/net/rpc/src/beacon/builder_market_tests.rs b/crates/net/rpc/src/beacon/builder_market_tests.rs index 981a0177e..8ad42edce 100644 --- a/crates/net/rpc/src/beacon/builder_market_tests.rs +++ b/crates/net/rpc/src/beacon/builder_market_tests.rs @@ -1060,29 +1060,48 @@ async fn valid_preferences_are_cached_and_published_once() { assert_eq!(world.network.proposer_preferences.lock().unwrap().len(), 1); } -/// The validator client's own signer and HTTP client against this node's -/// gossip rules: the domain `sign_proposer_preferences` picks (the fork of the -/// epoch before the proposal's) is the one `validate_proposer_preferences_gossip` -/// verifies under, and the JSON the client posts is what the route decodes. +/// The validator client's own path end to end against this node: it reads the +/// proposer duty and its v2 `dependent_root` from the served duties route, +/// signs with `SigningContext::sign_proposer_preferences`, and posts through +/// `HttpBeaconNode`, and the node's gossip rules accept it. That pins the +/// duties route's dependent root as the one the preference must carry, and the +/// JSON the client posts as the one the route decodes. /// -/// The fork-boundary case (a preference for the first gloas epoch, signed -/// under the fulu version) is not driven here: this world's schedule has gloas -/// at epoch 0 and a second world in the epoch before the fork would be a new -/// chain fixture. `signing.rs` pins that domain in a unit test. +/// It does not pin the fork-version choice: this world has gloas at epoch 0 +/// and the node accepts either version, so `signing.rs`'s unit test is the pin +/// for the domain at a fork boundary. #[tokio::test] async fn preferences_signed_by_the_validator_client_pass_this_nodes_gossip_rules() { + use ethlambda_blockchain::{SyncStatusController, metrics::SyncStatus}; use ethlambda_validator::{ - beacon_node::{BeaconNodeApi as _, http::HttpBeaconNode}, + beacon_node::{BeaconNodeApi as _, dto::parse_pubkey, http::HttpBeaconNode}, keys::ValidatorStore, signing::SigningContext, }; let world = World::new(); - let proposer = world.prepared().proposer; - let dependent_root = dependent_root_at(&world.state, world.head_root, SLOT) - .expect("the head's chain gives the slot a dependent root"); + let duties_routes = crate::beacon::validator::routes() + .with_state(world.store.clone()) + .layer(Extension(SyncStatusController::new(SyncStatus::Synced))); + let router = world.app(None).merge(duties_routes); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let bound = listener.local_addr().unwrap(); + tokio::spawn(async move { axum::serve(listener, router).await }); + let client = HttpBeaconNode::new(format!("http://{bound}")).unwrap(); + + // The duty and the dependent root the way the validator client reads them. + let duties = client + .proposer_duties_v2(compute_epoch_at_slot(SLOT)) + .await + .expect("the node serves v2 proposer duties"); + let duty = duties + .duties + .iter() + .find(|duty| duty.slot == SLOT) + .expect("the slot has a proposer"); + let proposer = duty.validator_index; let message = ProposerPreferences { - dependent_root, + dependent_root: duties.dependent_root, proposal_slot: SLOT, validator_index: proposer, fee_recipient: address(0xaa), @@ -1095,7 +1114,7 @@ async fn preferences_signed_by_the_validator_client_pass_this_nodes_gossip_rules .expect("the test secret is a valid key"); assert_eq!( pubkey, - world.state.validator(proposer).unwrap().pubkey, + parse_pubkey(&duty.pubkey).expect("the duty names a key"), "the test registry's key for the proposer" ); let context = SigningContext { @@ -1107,12 +1126,6 @@ async fn preferences_signed_by_the_validator_client_pass_this_nodes_gossip_rules .expect("signs"); let signed = SignedProposerPreferences { message, signature }; - let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); - let address = listener.local_addr().unwrap(); - let router = world.app(None); - tokio::spawn(async move { axum::serve(listener, router).await }); - let client = HttpBeaconNode::new(format!("http://{address}")).unwrap(); - client .submit_proposer_preferences( ethlambda_types::beacon::fork::ForkName::Gloas, @@ -1125,7 +1138,12 @@ async fn preferences_signed_by_the_validator_client_pass_this_nodes_gossip_rules *world.network.proposer_preferences.lock().unwrap(), vec![signed.clone()] ); - assert_eq!(world.market.preferences(SLOT, dependent_root), Some(signed)); + assert_eq!( + world + .market + .preferences(SLOT, signed.message.dependent_root), + Some(signed) + ); } #[tokio::test] diff --git a/crates/net/rpc/src/beacon/validator.rs b/crates/net/rpc/src/beacon/validator.rs index ee2f0bcde..5a472f50b 100644 --- a/crates/net/rpc/src/beacon/validator.rs +++ b/crates/net/rpc/src/beacon/validator.rs @@ -35,7 +35,9 @@ use serde::{Deserialize, Serialize}; use ethlambda_network_api::RpcToP2PRef; use ethlambda_state_transition::beacon::{ - fork_choice::{checkpoint_state, get_current_store_epoch, get_head_node}, + fork_choice::{ + checkpoint_state, compute_shuffling_dependent_slot, get_current_store_epoch, get_head_node, + }, gossip::attestation::compute_subnet_for_attestation, helpers::accessors::{CommitteeCacheExt as _, get_block_root_at_slot}, helpers::altair::compute_sync_committee_period, @@ -535,11 +537,11 @@ impl DependentRoot { /// definition names the genesis block where its subtraction would /// underflow, which is slot 0. fn slot(self, epoch: Epoch) -> Slot { - let epoch = match self { - Self::V1 => epoch, - Self::V2 => epoch.saturating_sub(1), - }; - compute_start_slot_at_epoch(epoch).saturating_sub(1) + match self { + Self::V1 => compute_start_slot_at_epoch(epoch).saturating_sub(1), + // The state transition's own definition, so the two cannot drift. + Self::V2 => compute_shuffling_dependent_slot(epoch), + } } } diff --git a/crates/validator/src/beacon_node/fallback.rs b/crates/validator/src/beacon_node/fallback.rs index 933ea34b7..32cca315b 100644 --- a/crates/validator/src/beacon_node/fallback.rs +++ b/crates/validator/src/beacon_node/fallback.rs @@ -86,6 +86,32 @@ impl FallbackBeaconNode { /// answered, for the calls whose follow-up must go to the same node. The /// logging and the final `warn!` live here so the two cannot drift. async fn try_each_at<'p, T, F>(&'p self, what: &'static str, call: F) -> Result<(usize, T)> + where + F: Fn(&'p B) -> BoxFuture<'p, Result>, + { + self.try_each_at_inner(what, true, call).await + } + + /// [`Self::try_each`] without the final `warn!`, for a caller whose own + /// duty did not depend on the answer and which reports the failure itself + /// (the counterpart of [`Self::try_all`]'s `all_failed: None`). A node that + /// is syncing, or lacks the endpoint, would otherwise make this warn on + /// every call for as long as it lasts. + async fn try_each_quiet<'p, T, F>(&'p self, what: &'static str, call: F) -> Result + where + F: Fn(&'p B) -> BoxFuture<'p, Result>, + { + self.try_each_at_inner(what, false, call) + .await + .map(|(_, value)| value) + } + + async fn try_each_at_inner<'p, T, F>( + &'p self, + what: &'static str, + warn_when_all_fail: bool, + call: F, + ) -> Result<(usize, T)> where F: Fn(&'p B) -> BoxFuture<'p, Result>, { @@ -106,7 +132,9 @@ impl FallbackBeaconNode { let detail = last .map(|err| err.to_string()) .unwrap_or_else(|| "no nodes configured".to_string()); - warn!(%what, %detail, "All configured beacon nodes failed; duty likely missed"); + if warn_when_all_fail { + warn!(%what, %detail, "All configured beacon nodes failed; duty likely missed"); + } Err(Error::AllBeaconNodesFailed(detail)) } @@ -257,7 +285,9 @@ impl BeaconNodeApi for FallbackBeaconNode { } async fn proposer_duties_v2(&self, epoch: Epoch) -> Result { - self.try_each("proposer_duties_v2", |node| node.proposer_duties_v2(epoch)) + // Quiet: the preferences service reports its own failure once per + // epoch, and no duty depends on this answer. + self.try_each_quiet("proposer_duties_v2", |node| node.proposer_duties_v2(epoch)) .await } @@ -358,10 +388,8 @@ impl BeaconNodeApi for FallbackBeaconNode { self.try_all( "submit_proposer_preferences", "it will not gossip these proposer preferences", - Some( - "no node took this client's proposer preferences; builders' bids for those \ - slots are ignored, and from gloas a node may not know the fee recipient", - ), + // The service reports it, per preference, with what it costs. + None, |node| node.submit_proposer_preferences(fork, preferences), ) .await diff --git a/crates/validator/src/beacon_node/mock.rs b/crates/validator/src/beacon_node/mock.rs index c56287708..bd3747f11 100644 --- a/crates/validator/src/beacon_node/mock.rs +++ b/crates/validator/src/beacon_node/mock.rs @@ -69,6 +69,10 @@ pub struct MockBeaconNode { /// Every proposer preferences batch `submit_proposer_preferences` /// received, with the fork it was sent under, in call order. pub submitted_preferences: Mutex)>>, + /// Proposal slots whose preference the mock refuses, as a node does one it + /// holds with other content: a call carrying any of them is an `Err` and + /// records nothing, like the node's 400 for the whole call. + pub refused_preference_slots: Mutex>, pub attestation_data: Option, /// The slot and proposer the mock will produce a block for. `None` makes /// `produce_block` fail, the way an unset `attestation_data` does. @@ -291,8 +295,9 @@ impl MockBeaconNode { self } - /// How many times `proposer_duties_v2` has been called, successes and - /// failures alike. + /// How many times `proposer_duties_v2` has been called and got past the + /// configured failures, answers and misses alike. A call `failures` or + /// `fail_everything` rejects is not counted. pub fn proposer_duties_v2_call_count(&self) -> usize { *self.proposer_duties_v2_calls.lock().expect("lock") } @@ -579,8 +584,9 @@ impl MockBeaconNode { *self.validator_indices_calls.lock().expect("lock") } - /// How many times `proposer_duties` has been called, successes and - /// failures alike. + /// How many times `proposer_duties` has been called and got past the + /// configured failures, answers and misses alike. A call `failures` or + /// `fail_everything` rejects is not counted. pub fn proposer_duties_call_count(&self) -> usize { *self.proposer_duties_calls.lock().expect("lock") } @@ -843,6 +849,19 @@ impl BeaconNodeApi for MockBeaconNode { preferences: &[gloas::SignedProposerPreferences], ) -> Result<()> { self.guard("submit_proposer_preferences")?; + let refused = self.refused_preference_slots.lock().expect("lock").clone(); + if let Some(entry) = preferences + .iter() + .find(|entry| refused.contains(&entry.message.proposal_slot)) + { + return Err(Error::BeaconNodeStatus { + status: 400, + body: format!( + "preference for slot {} refused", + entry.message.proposal_slot + ), + }); + } self.submitted_preferences .lock() .expect("lock") diff --git a/crates/validator/src/beacon_node/mod.rs b/crates/validator/src/beacon_node/mod.rs index 5c8c189de..4eb8d8026 100644 --- a/crates/validator/src/beacon_node/mod.rs +++ b/crates/validator/src/beacon_node/mod.rs @@ -426,13 +426,16 @@ pub trait BeaconNodeApi: Send + Sync { /// Submit signed proposer preferences, which the node validates with the /// `proposer_preferences` gossip rules and publishes. `fork` is the - /// `Eth-Consensus-Version`: the fork of the current epoch. + /// `Eth-Consensus-Version`: the fork the preferences belong to, that of + /// their proposal epoch. /// /// A 400 naming per-index failures is an `Err`: the whole call failed for - /// the caller's purposes. The caller re-submits the entries next slot, and - /// the node answers per entry, so the ones that already went out are - /// deduplicated by the node. State on the node's gossip, so failover sends - /// it to every node. + /// the caller's purposes, although the node may have published the entries + /// that passed. A node only short-circuits an identical entry, so one it + /// holds for the same `(slot, dependent_root)` with other content fails + /// every call that carries it. The caller therefore sends one preference + /// per call. State on the node's gossip, so failover sends it to every + /// node. async fn submit_proposer_preferences( &self, fork: ForkName, diff --git a/crates/validator/src/lib.rs b/crates/validator/src/lib.rs index d95df82d5..3eb4e19e4 100644 --- a/crates/validator/src/lib.rs +++ b/crates/validator/src/lib.rs @@ -50,6 +50,8 @@ pub mod signing; pub mod slot_clock; pub mod subscriptions; pub mod sync_committee; +#[cfg(test)] +mod test_support; pub use error::{Error, Result}; @@ -107,9 +109,10 @@ pub struct ValidatorConfig { /// and accept a blinded block when the builder's bid wins. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct BuilderProposals { - /// The `builder_boost_factor` sent with every pre-gloas block request: - /// 100 compares the bid and the local payload as they are; `u64::MAX` - /// takes the builder whenever there is a bid. + /// The `builder_boost_factor` sent with every block request (in the query + /// before gloas, in `produceBlockV4`'s body from gloas): 100 compares the + /// bid and the local payload as they are; `u64::MAX` takes the builder + /// whenever there is a bid. pub boost_factor: u64, } @@ -352,6 +355,13 @@ pub async fn run(config: ValidatorConfig) -> Result<()> { } } + // First in the slot, ahead of the proposal: the preferences for the + // slots after this one must be out before those slots start (gossip + // IGNOREs a started slot), and a poll queued behind a duty could land + // late. Off the slot loop, since it is single-flight and a slow node + // must not delay this slot's duties. + tokio::spawn(preferences.clone().poll(slot, store.clone())); + // A block is due now, at the boundary this iteration woke on. // // Cloned out of `duties` so the borrow ends before the await: the duty @@ -391,10 +401,6 @@ pub async fn run(config: ValidatorConfig) -> Result<()> { if let Some(registration) = ®istration { tokio::spawn(registration.clone().register_changed(epoch, store.clone())); } - - // Likewise off the slot loop: the poll is single-flight, and a slow - // node must not delay the next slot's duties. - tokio::spawn(preferences.clone().poll(slot, store.clone())); } } diff --git a/crates/validator/src/preferences.rs b/crates/validator/src/preferences.rs index 667a88875..ec87a925c 100644 --- a/crates/validator/src/preferences.rs +++ b/crates/validator/src/preferences.rs @@ -22,8 +22,9 @@ //! # The signing domain //! //! See [`SigningContext::sign_proposer_preferences`]: the gossip rule's domain, -//! at the fork of the epoch before the proposal's, which is not the validator -//! guide's head-state one in the first epoch of a fork. +//! at the fork of the epoch before the proposal's. The validator guide's +//! head-state signer differs from it only for a proposal in the first epoch of +//! a fork, signed during that epoch. //! //! # Why every slot //! @@ -46,15 +47,25 @@ //! replaced for the same slot and root. A fee recipient changed after a //! preference went out applies from the next dependent root onward only. //! +//! # One preference per request +//! +//! A node answers 400 for a whole call when any entry in it fails, and one it +//! holds for the same `(slot, dependent_root)` with other content is such a +//! failure (a restart after a fee recipient change, say). Batching would then +//! fail every entry every slot until that slot passed, so each preference is +//! sent alone, and each success is remembered on its own. There are only a +//! handful per epoch. +//! //! # Failures are never propagated //! //! A missed preference costs a builder bid and, from gloas, possibly the -//! node's knowledge of the fee recipient, never a duty. A failed send is logged -//! and counted, and the next slot retries. +//! node's knowledge of the fee recipient, never a duty. A failed send, or a +//! failed duties fetch, is logged (the fetch once per epoch) and counted, and +//! the next slot retries. use std::collections::HashSet; -use std::sync::Arc; use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::{Arc, Mutex, MutexGuard}; use ethlambda_types::beacon::containers::gloas::{ProposerPreferences, SignedProposerPreferences}; use ethlambda_types::beacon::fork::ForkName; @@ -88,15 +99,26 @@ pub struct PreferencesService { /// `(proposal_slot, dependent_root)` pairs a node accepted, so a /// preference is signed and sent once per dependent root. A reorg that /// moves the root sends the slot again; pruned below the current epoch. - published: std::sync::Mutex>, + published: Mutex>, /// Epochs whose missing fee recipients were already warned about, so the /// warning is once per epoch rather than once per slot. Pruned like /// `published`. - warned_epochs: std::sync::Mutex>, + no_fee_warned: Mutex>, + /// Epochs whose failed duties fetch was already warned about, for the same + /// reason: a syncing node, or one without v2, fails every slot. + fetch_warned: Mutex>, /// One poll at a time: a slow node must not stack polls up. in_flight: AtomicBool, } +/// Lock `mutex`, recovering a poisoned one: every write here is one set +/// operation, so the data behind it is never half-changed. +fn lock(mutex: &Mutex) -> MutexGuard<'_, T> { + mutex + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) +} + impl PreferencesService { pub fn new( beacon_node: Arc, @@ -107,8 +129,9 @@ impl PreferencesService { beacon_node, context, settings, - published: std::sync::Mutex::new(HashSet::new()), - warned_epochs: std::sync::Mutex::new(HashSet::new()), + published: Mutex::new(HashSet::new()), + no_fee_warned: Mutex::new(HashSet::new()), + fetch_warned: Mutex::new(HashSet::new()), in_flight: AtomicBool::new(false), } } @@ -128,31 +151,24 @@ impl PreferencesService { // Snapshotted with the guard dropped at once: nothing below awaits a // node while holding the store. let pubkeys: HashSet = store.read().await.pubkeys().into_iter().collect(); - let fork = self.context.config.fork_at_epoch(epoch); for target in [epoch, epoch + 1] { if self.context.config.fork_at_epoch(target) < ForkName::Gloas { continue; } - self.publish_epoch(fork, current_slot, target, &pubkeys, &store) + self.publish_epoch(current_slot, target, &pubkeys, &store) .await; } let epoch_start = epoch * preset::SLOTS_PER_EPOCH; - self.published - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .retain(|(slot, _)| *slot >= epoch_start); - self.warned_epochs - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .retain(|warned| *warned >= epoch); + lock(&self.published).retain(|(slot, _)| *slot >= epoch_start); + lock(&self.no_fee_warned).retain(|warned| *warned >= epoch); + lock(&self.fetch_warned).retain(|warned| *warned >= epoch); } /// One target epoch: fetch its v2 duties, sign what is still owed and send it. async fn publish_epoch( &self, - fork: ForkName, current_slot: Slot, target: Epoch, pubkeys: &HashSet, @@ -162,6 +178,15 @@ impl PreferencesService { Ok(duties) => duties, Err(err) => { debug!(epoch = target, %err, "Could not fetch proposer duties for preferences"); + crate::metrics::inc_proposer_preferences_failures(); + if lock(&self.fetch_warned).insert(target) { + warn!( + epoch = target, + %err, + "No beacon node answered the v2 proposer duties; proposer preferences \ + for this epoch wait for a node that does" + ); + } return; } }; @@ -179,12 +204,7 @@ impl PreferencesService { if !pubkeys.contains(&pubkey) { continue; } - let already = self - .published - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .contains(&(duty.slot, duties.dependent_root)); - if already { + if lock(&self.published).contains(&(duty.slot, duties.dependent_root)) { continue; } let Some(fee_recipient) = self.settings.fee_recipient(&pubkey) else { @@ -203,26 +223,19 @@ impl PreferencesService { )); } - if no_fee_recipient > 0 { - let first_warning = self - .warned_epochs - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .insert(target); - if first_warning { - warn!( - epoch = target, - skipped = no_fee_recipient, - "Validators without a fee recipient publish no proposer preferences" - ); - } + if no_fee_recipient > 0 && lock(&self.no_fee_warned).insert(target) { + warn!( + epoch = target, + skipped = no_fee_recipient, + "Validators without a fee recipient publish no proposer preferences" + ); } if owed.is_empty() { return; } // At most a few per epoch, so signing inline is cheap; the read guard - // is released before the submit. + // is released before the submits. let mut signed = Vec::with_capacity(owed.len()); { let store = store.read().await; @@ -243,33 +256,47 @@ impl PreferencesService { } } } - if signed.is_empty() { - return; - } - match self - .beacon_node - .submit_proposer_preferences(fork, &signed) - .await - { - Ok(()) => { - let count = signed.len(); - self.published - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .extend( - signed.iter().map(|entry| { - (entry.message.proposal_slot, entry.message.dependent_root) - }), + // The target epoch's fork, which is what the submitted objects belong + // to (always gloas here). + let fork = self.context.config.fork_at_epoch(target); + let mut published = 0usize; + for entry in signed { + // One request per preference: a node answers 400 for the whole + // call when any entry fails, and the one it refuses (for a + // `(slot, root)` it holds with other content, say) would + // otherwise keep the others from ever being marked published. + let key = (entry.message.proposal_slot, entry.message.dependent_root); + match self + .beacon_node + .submit_proposer_preferences(fork, std::slice::from_ref(&entry)) + .await + { + Ok(()) => { + lock(&self.published).insert(key); + published += 1; + } + Err(err) => { + warn!( + epoch = target, + slot = key.0, + validator = entry.message.validator_index, + %err, + "No beacon node accepted a proposer preference; builders' bids for that \ + slot are ignored, and from gloas a node may not know the fee recipient" ); - crate::metrics::inc_proposer_preferences_published(count); - info!(epoch = target, count, "Proposer preferences published"); - } - Err(err) => { - warn!(epoch = target, count = signed.len(), %err, "No beacon node accepted the proposer preferences"); - crate::metrics::inc_proposer_preferences_failures(); + crate::metrics::inc_proposer_preferences_failures(); + } } } + if published > 0 { + crate::metrics::inc_proposer_preferences_published(published); + info!( + epoch = target, + count = published, + "Proposer preferences published" + ); + } } } @@ -278,9 +305,10 @@ mod tests { use super::*; use crate::beacon_node::dto::ProposerDutyDto; use crate::beacon_node::mock::MockBeaconNode; + use crate::test_support::{settings, store_with_keys}; use ethlambda_types::beacon::config::Config; use ethlambda_types::beacon::constants::DOMAIN_PROPOSER_PREFERENCES; - use ethlambda_types::beacon::primitives::{Bytes32, H160, HashTreeRoot as _}; + use ethlambda_types::beacon::primitives::{H160, HashTreeRoot as _}; use ethlambda_types::beacon::signing::{compute_domain, compute_signing_root}; const GLOAS_EPOCH: Epoch = 500_000; @@ -295,20 +323,6 @@ mod tests { }) } - /// `count` distinct keys. The secrets are small integers, which are valid - /// BLS scalars, so no keystore is needed. - fn store_with_keys(count: usize) -> (ValidatorStore, Vec) { - let mut store = ValidatorStore::new(); - let pubkeys = (0..count) - .map(|index| { - let mut secret = [0u8; 32]; - secret[24..].copy_from_slice(&(index as u64 + 1).to_be_bytes()); - store.insert_secret("test", &secret).expect("inserts") - }) - .collect(); - (store, pubkeys) - } - fn duty(pubkey: &BlsPubkey, validator_index: u64, slot: Slot) -> ProposerDutyDto { ProposerDutyDto { pubkey: encode_hex(&pubkey.0), @@ -317,10 +331,6 @@ mod tests { } } - fn settings(fee_recipient: Option) -> Arc { - Arc::new(ProposerSettings::new(Bytes32::ZERO, fee_recipient)) - } - fn service( node: MockBeaconNode, settings: Arc, @@ -345,16 +355,19 @@ mod tests { let context = context(); let proposal_epoch = compute_epoch_at_slot(entry.message.proposal_slot); - let domain = context.domain(DOMAIN_PROPOSER_PREFERENCES, proposal_epoch - 1); - assert_eq!( - domain, - compute_domain( - DOMAIN_PROPOSER_PREFERENCES, - context - .config - .fork_version(context.config.fork_at_epoch(proposal_epoch - 1)), - context.genesis_validators_root, - ) + // Spelled out rather than asked of `SigningContext::domain`: the + // first gloas epoch's preference verifies under fulu's version (the + // fork of the epoch before it), every later one under gloas's. + let version = if proposal_epoch == GLOAS_EPOCH { + context.config.fulu_fork_version + } else { + assert!(proposal_epoch > GLOAS_EPOCH); + context.config.gloas_fork_version + }; + let domain = compute_domain( + DOMAIN_PROPOSER_PREFERENCES, + version, + context.genesis_validators_root, ); let root = compute_signing_root(entry.message.hash_tree_root(), domain); let pk = PublicKey::from_bytes(&pubkey.0).expect("valid pubkey"); @@ -392,7 +405,7 @@ mod tests { service.poll(current, store).await; let sent = submitted(&node); - assert_eq!(sent.len(), 2, "one send per target epoch"); + assert_eq!(sent.len(), 2, "one request per preference"); let mut entries: Vec<_> = sent .iter() .inspect(|(fork, _)| assert_eq!(*fork, ForkName::Gloas)) @@ -477,7 +490,7 @@ mod tests { .await; let sent = submitted(&node); assert_eq!(sent.len(), 1); - assert_eq!(sent[0].0, ForkName::Fulu, "the current epoch's fork"); + assert_eq!(sent[0].0, ForkName::Gloas, "the target epoch's fork"); assert_eq!(sent[0].1[0].message.proposal_slot, slot_in(GLOAS_EPOCH, 10)); assert_eq!(node.proposer_duties_v2_call_count(), 1); @@ -552,6 +565,76 @@ mod tests { assert_eq!(sent[0].1[0].message.validator_index, 4); } + /// A node that refuses one preference (it holds another for that slot and + /// root) must not keep the others from going out and being remembered, nor + /// make them re-sent every slot. + #[tokio::test] + async fn a_refused_preference_does_not_block_the_others() { + let (store, pubkeys) = store_with_keys(2); + let store = Arc::new(RwLock::new(store)); + let refused = slot_in(GLOAS_EPOCH, 10); + let accepted = slot_in(GLOAS_EPOCH, 11); + let node = MockBeaconNode::new().with_proposers_v2( + GLOAS_EPOCH, + Root::repeat_byte(0x11), + vec![ + duty(&pubkeys[0], 3, refused), + duty(&pubkeys[1], 4, accepted), + ], + ); + node.refused_preference_slots + .lock() + .expect("lock") + .push(refused); + let (service, node) = service(node, settings(Some(FEE_RECIPIENT))); + let current = slot_in(GLOAS_EPOCH, 1); + + service.clone().poll(current, store.clone()).await; + let sent = submitted(&node); + assert_eq!(sent.len(), 1, "only the accepted one was recorded"); + assert_eq!(sent[0].1[0].message.proposal_slot, accepted); + + // Next poll: the accepted one is not re-sent, the refused one is + // tried again (and refused again). + service.clone().poll(current + 1, store.clone()).await; + assert_eq!(submitted(&node).len(), 1); + + node.refused_preference_slots.lock().expect("lock").clear(); + service.poll(current + 2, store).await; + let sent = submitted(&node); + assert_eq!(sent.len(), 2); + assert_eq!(sent[1].1[0].message.proposal_slot, refused); + } + + /// A failed duties fetch is warned about once per target epoch, and the + /// next poll that gets an answer publishes. + #[tokio::test] + async fn a_failed_duties_fetch_is_retried_and_warned_once_per_epoch() { + let (store, pubkeys) = store_with_keys(1); + let store = Arc::new(RwLock::new(store)); + let node = MockBeaconNode::new().with_proposers_v2( + GLOAS_EPOCH, + Root::repeat_byte(0x11), + vec![duty(&pubkeys[0], 3, slot_in(GLOAS_EPOCH, 10))], + ); + node.failures + .lock() + .expect("lock") + .insert("proposer_duties_v2", "syncing".to_string()); + let (service, node) = service(node, settings(Some(FEE_RECIPIENT))); + let current = slot_in(GLOAS_EPOCH, 1); + + service.clone().poll(current, store.clone()).await; + service.clone().poll(current + 1, store.clone()).await; + assert!(submitted(&node).is_empty()); + let warned: Vec = lock(&service.fetch_warned).iter().copied().collect(); + assert_eq!(warned.len(), 2, "one entry per target epoch, not per poll"); + + node.failures.lock().expect("lock").clear(); + service.poll(current + 2, store).await; + assert_eq!(submitted(&node).len(), 1); + } + #[tokio::test] async fn a_failed_submit_is_retried_next_poll() { let (store, pubkeys) = store_with_keys(1); diff --git a/crates/validator/src/registration.rs b/crates/validator/src/registration.rs index c9d5a717d..0918096ff 100644 --- a/crates/validator/src/registration.rs +++ b/crates/validator/src/registration.rs @@ -318,8 +318,9 @@ impl RegistrationService { mod tests { use super::*; use crate::beacon_node::mock::MockBeaconNode; + use crate::test_support::{settings, store_with_keys}; use ethlambda_types::beacon::config::Config; - use ethlambda_types::beacon::primitives::{Bytes32, H160, Root}; + use ethlambda_types::beacon::primitives::{H160, Root}; fn context(config: Config) -> Arc { Arc::new(SigningContext { @@ -328,24 +329,6 @@ mod tests { }) } - /// `count` distinct keys. The secrets are small integers, which are valid - /// BLS scalars, so no keystore is needed. - fn store_with_keys(count: usize) -> (ValidatorStore, Vec) { - let mut store = ValidatorStore::new(); - let pubkeys = (0..count) - .map(|index| { - let mut secret = [0u8; 32]; - secret[24..].copy_from_slice(&(index as u64 + 1).to_be_bytes()); - store.insert_secret("test", &secret).expect("inserts") - }) - .collect(); - (store, pubkeys) - } - - fn settings(fee_recipient: Option) -> Arc { - Arc::new(ProposerSettings::new(Bytes32::ZERO, fee_recipient)) - } - fn service( config: Config, settings: Arc, diff --git a/crates/validator/src/signing.rs b/crates/validator/src/signing.rs index d43a36029..2ece1aca3 100644 --- a/crates/validator/src/signing.rs +++ b/crates/validator/src/signing.rs @@ -326,8 +326,12 @@ impl SigningContext { /// against: `get_domain(state, .., proposal_epoch)` on the dependent /// block's state advanced to the lookahead start, whose fork is the one /// in force an epoch before the proposal. The validator guide signs with - /// the head state instead; the two differ only in the first epoch of a - /// fork, where this one is the one peers accept. Not slashable. + /// the head state (`get_domain` reads `state.fork`, not the schedule); + /// the two agree everywhere except for a proposal in the first epoch of a + /// fork, signed during that epoch: the head state already carries the new + /// version, while the gossip rule's state, at the start of the previous + /// epoch, still carries the old one. This signer takes the gossip rule's, + /// which is the one peers verify. Not slashable. pub fn sign_proposer_preferences( &self, store: &ValidatorStore, @@ -643,9 +647,10 @@ mod tests { } /// The first gloas epoch's preference is signed at the fork of the epoch - /// before it (fulu), which is what the gossip rule verifies against, and - /// not under the gloas version the validator guide's head-state signer - /// would use. + /// before it (fulu), which is what the gossip rule verifies against. The + /// validator guide's head-state signer would give gloas for a preference + /// signed during that epoch, which the gossip rule's state (still fulu) + /// would reject. #[test] fn a_preference_for_the_first_gloas_epoch_is_signed_under_the_fulu_version() { let (store, pubkey) = store_with_key(); diff --git a/crates/validator/src/test_support.rs b/crates/validator/src/test_support.rs new file mode 100644 index 000000000..d9216b9ab --- /dev/null +++ b/crates/validator/src/test_support.rs @@ -0,0 +1,27 @@ +//! Helpers the service tests share. + +use std::sync::Arc; + +use ethlambda_types::beacon::primitives::{BlsPubkey, Bytes32, H160}; + +use crate::keys::ValidatorStore; +use crate::proposer_settings::ProposerSettings; + +/// `count` distinct keys. The secrets are small integers, which are valid BLS +/// scalars, so no keystore is needed. +pub(crate) fn store_with_keys(count: usize) -> (ValidatorStore, Vec) { + let mut store = ValidatorStore::new(); + let pubkeys = (0..count) + .map(|index| { + let mut secret = [0u8; 32]; + secret[24..].copy_from_slice(&(index as u64 + 1).to_be_bytes()); + store.insert_secret("test", &secret).expect("inserts") + }) + .collect(); + (store, pubkeys) +} + +/// Settings with `fee_recipient` as the default one and the default graffiti. +pub(crate) fn settings(fee_recipient: Option) -> Arc { + Arc::new(ProposerSettings::new(Bytes32::ZERO, fee_recipient)) +} diff --git a/docs/cli.md b/docs/cli.md index f6c480485..1e387769f 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -290,7 +290,7 @@ implementation's, so none of the [common flags](#common-flags) apply to it. | `--suggested-fee-recipient` | none | Execution address to receive block rewards, `0x`-prefixed, for every validator the keymanager API gives no address of its own. Optional, and startup warns when it is absent: without it the beacon node picks an address, and it will not be yours | | `--graffiti` | empty | Text for the graffiti field of proposed blocks, for every validator the keymanager API gives no graffiti of its own. At most 32 bytes as UTF-8, right-padded with zeros. Refused rather than truncated if longer. An ethlambda beacon node appends both clients' versions to it (see [`rpc.md`](rpc.md#validator-endpoints)) | | `--enable-keymanager` | off | Serve the keymanager API: keystores, plus each validator's own fee recipient, graffiti and gas limit. Off by default because it mutates key material | -| `--builder-proposals` | off | Propose builder blocks: register each validator with the beacon node's builder (an ethlambda node: `--builder`) and accept a blinded block when the builder's bid wins. Fulu and earlier only; from gloas the builder market works through proposer preferences instead | +| `--builder-proposals` | off | Propose builder blocks: register each validator with the beacon node's builder (an ethlambda node: `--builder`) and accept a blinded block when the builder's bid wins. Registrations and blinded blocks are fulu and earlier only; from gloas the builder market works through proposer preferences, and the boost factor weighs p2p bids in `produceBlockV4` | | `--builder-boost-factor` | `100` | How a builder's bid is weighed against the local payload: the local payload wins iff `local >= factor * bid / 100`. `0` prefers the local payload unless it cannot be built. From gloas it is also sent in `produceBlockV4`'s body, so a pooled p2p bid can win on value; without `--builder-proposals` the body carries `0`. Requires `--builder-proposals`; conflicts with `--prefer-builder-proposals` | | `--prefer-builder-proposals` | off | Take the builder's block whenever there is a valid bid, unless the execution client asks to override the builder (sends `builder_boost_factor = u64::MAX`). Requires `--builder-proposals`; conflicts with `--builder-boost-factor` | diff --git a/docs/spec_deviations.md b/docs/spec_deviations.md index 15fd6ec82..b1456d536 100644 --- a/docs/spec_deviations.md +++ b/docs/spec_deviations.md @@ -722,10 +722,13 @@ so the fork version it uses is the one in force an epoch before the proposal's. - **ethlambda:** the validator client signs at the epoch before the proposal's (`SigningContext::sign_proposer_preferences`), which is the domain the gossip rule verifies. -- **Why:** the two agree except in the first epoch of a fork, where the head - state still carries the old fork version and a proposal's epoch resolves to - the new one, so the guide's signature would fail the gossip rule that peers - apply. This node's own `proposer_preferences` endpoint accepts both. +- **Why:** `get_domain` reads `state.fork`, not the schedule, so the two agree + everywhere except for a proposal in the first epoch of a fork, signed during + that epoch. There the head state already carries the new fork version, while + the gossip rule's state (at the start of the previous epoch) still carries the + old one, so the guide's signature would fail the rule peers apply. During the + epoch before the fork both give the old version. This node's own + `proposer_preferences` endpoint accepts both. - **Published unconditionally,** not gated on `--builder-proposals`, since from gloas the preferences carry every proposal's fee recipient. Validators with no fee recipient are skipped (the field is required), and a preference cannot be From ff91d4ebcae24c9d988ca5ecb535d94dbf229b40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Mon, 26 Oct 2026 18:36:13 -0300 Subject: [PATCH 08/16] fix(validator): warn once per refused proposer preference A preference a node keeps refusing is retried every slot until its slot passes, which warned up to two epochs of slots per entry. Warn on the first refusal of each (slot, dependent root), log repeats at debug, and keep counting every failure. Reword the failures metric to what it now counts. --- crates/validator/src/metrics.rs | 9 ++++--- crates/validator/src/preferences.rs | 40 +++++++++++++++++++++++------ docs/metrics.md | 2 +- 3 files changed, 38 insertions(+), 13 deletions(-) diff --git a/crates/validator/src/metrics.rs b/crates/validator/src/metrics.rs index 92fc44b2f..4367cedb3 100644 --- a/crates/validator/src/metrics.rs +++ b/crates/validator/src/metrics.rs @@ -144,13 +144,14 @@ static PROPOSER_PREFERENCES_PUBLISHED_TOTAL: LazyLock = LazyLock::ne .unwrap() }); -/// Proposer preference batches no beacon node accepted. The next slot retries, -/// so this is a warning sign: until one is accepted, builders' bids for the -/// slot are ignored. +/// Proposer preferences no beacon node accepted, one count per preference per +/// attempt, plus failed v2 proposer-duty fetches. The next slot retries, so +/// this is a warning sign: until one is accepted, builders' bids for the slot +/// are ignored. static PROPOSER_PREFERENCES_FAILURES_TOTAL: LazyLock = LazyLock::new(|| { register_int_counter!( "ethlambda_validator_proposer_preferences_failures_total", - "Proposer preference batches no beacon node accepted" + "Proposer preferences no beacon node accepted, plus failed v2 duty fetches" ) .unwrap() }); diff --git a/crates/validator/src/preferences.rs b/crates/validator/src/preferences.rs index ec87a925c..72ec3b501 100644 --- a/crates/validator/src/preferences.rs +++ b/crates/validator/src/preferences.rs @@ -104,6 +104,10 @@ pub struct PreferencesService { /// warning is once per epoch rather than once per slot. Pruned like /// `published`. no_fee_warned: Mutex>, + /// `(proposal_slot, dependent_root)` entries whose refusal was already + /// warned about, so a persistently refused one warns once rather than every + /// slot until its slot passes. Pruned with `published`. + refusal_warned: Mutex>, /// Epochs whose failed duties fetch was already warned about, for the same /// reason: a syncing node, or one without v2, fails every slot. fetch_warned: Mutex>, @@ -131,6 +135,7 @@ impl PreferencesService { settings, published: Mutex::new(HashSet::new()), no_fee_warned: Mutex::new(HashSet::new()), + refusal_warned: Mutex::new(HashSet::new()), fetch_warned: Mutex::new(HashSet::new()), in_flight: AtomicBool::new(false), } @@ -162,6 +167,7 @@ impl PreferencesService { let epoch_start = epoch * preset::SLOTS_PER_EPOCH; lock(&self.published).retain(|(slot, _)| *slot >= epoch_start); + lock(&self.refusal_warned).retain(|(slot, _)| *slot >= epoch_start); lock(&self.no_fee_warned).retain(|warned| *warned >= epoch); lock(&self.fetch_warned).retain(|warned| *warned >= epoch); } @@ -277,14 +283,27 @@ impl PreferencesService { published += 1; } Err(err) => { - warn!( - epoch = target, - slot = key.0, - validator = entry.message.validator_index, - %err, - "No beacon node accepted a proposer preference; builders' bids for that \ - slot are ignored, and from gloas a node may not know the fee recipient" - ); + // A refused entry is tried again every slot until its slot + // passes, so only its first refusal is a warning. + if lock(&self.refusal_warned).insert(key) { + warn!( + epoch = target, + slot = key.0, + validator = entry.message.validator_index, + %err, + "No beacon node accepted a proposer preference; builders' bids for \ + that slot are ignored, and from gloas a node may not know the fee \ + recipient" + ); + } else { + debug!( + epoch = target, + slot = key.0, + validator = entry.message.validator_index, + %err, + "A proposer preference was refused again" + ); + } crate::metrics::inc_proposer_preferences_failures(); } } @@ -598,6 +617,11 @@ mod tests { // tried again (and refused again). service.clone().poll(current + 1, store.clone()).await; assert_eq!(submitted(&node).len(), 1); + assert_eq!( + *lock(&service.refusal_warned), + HashSet::from([(refused, Root::repeat_byte(0x11))]), + "two refusals of one entry leave one warn-path record" + ); node.refused_preference_slots.lock().expect("lock").clear(); service.poll(current + 2, store).await; diff --git a/docs/metrics.md b/docs/metrics.md index 6439c15ea..1788fd5a3 100644 --- a/docs/metrics.md +++ b/docs/metrics.md @@ -744,7 +744,7 @@ and absent is not the same as zero. | `ethlambda_validator_builder_registrations_sent_total` | Counter | Builder registrations a beacon node accepted (`--builder-proposals`) | | `ethlambda_validator_builder_registration_failures_total` | Counter | Registration batches no beacon node accepted. A missed registration costs at most a builder block, never a duty | | `ethlambda_validator_proposer_preferences_published_total` | Counter | Signed proposer preferences a beacon node accepted, counted per preference. Published for every proposal from the epoch before gloas, whether or not `--builder-proposals` is set | -| `ethlambda_validator_proposer_preferences_failures_total` | Counter | Proposer preference batches no beacon node accepted. The next slot retries; until one is accepted, builders' bids for those slots are ignored | +| `ethlambda_validator_proposer_preferences_failures_total` | Counter | Proposer preferences a beacon node refused or failed to take (one count per preference per attempt), plus failed v2 proposer-duty fetches. The next slot retries; until a preference is accepted, builders' bids for its slot are ignored | | `ethlambda_validator_blinded_blocks_proposed_total` | Counter | Blinded blocks a beacon node handed to its builder. A subset of `blocks_proposed_total` | | `ethlambda_validator_blinded_blocks_unconfirmed_total` | Counter | Blinded blocks the beacon node did not confirm (a 5xx or no answer), which the relay may still have published. Not retried, since a second block for the slot would be an equivocation | | `ethlambda_validator_block_publication_delay_seconds` | Histogram | Slot start to block accepted. Its buckets are tighter than the attestation histogram's, because a block is due at the slot boundary rather than a third of the way in | From a54c58c45d8baa675afdb79220b3fe6347d50874 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Mon, 26 Oct 2026 18:39:56 -0300 Subject: [PATCH 09/16] fix(validator): keep refused proposer preferences quiet per node and split the fetch counter try_all warned per node on every refusal, so a preference a node kept refusing still warned every slot despite the per-entry dedup; add a quiet variant for it. Poll after the proposal rather than before, so the next epoch duties fetch does not compete with block production. Count failed v2 duty fetches on their own counter, since a syncing node is not a refused preference. --- crates/validator/src/beacon_node/fallback.rs | 45 ++++++++++++++++++-- crates/validator/src/lib.rs | 16 ++++--- crates/validator/src/metrics.rs | 24 +++++++++-- crates/validator/src/preferences.rs | 26 +++++++++-- docs/metrics.md | 3 +- 5 files changed, 94 insertions(+), 20 deletions(-) diff --git a/crates/validator/src/beacon_node/fallback.rs b/crates/validator/src/beacon_node/fallback.rs index 32cca315b..90359e9b1 100644 --- a/crates/validator/src/beacon_node/fallback.rs +++ b/crates/validator/src/beacon_node/fallback.rs @@ -170,6 +170,39 @@ impl FallbackBeaconNode { all_failed: Option<&'static str>, call: F, ) -> Result<()> + where + F: Fn(&'p B) -> BoxFuture<'p, Result<()>>, + { + self.try_all_inner(what, node_effect, all_failed, true, call) + .await + } + + /// [`Self::try_all`] logging each node's refusal at `debug!` instead of + /// `warn!`, for a call the caller retries every slot and reports itself + /// (once per entry): a per-node warning here would repeat on every retry + /// for as long as the refusal lasts. Always without an `all_failed` + /// message, for the same reason. + async fn try_all_quiet<'p, F>( + &'p self, + what: &'static str, + node_effect: &'static str, + call: F, + ) -> Result<()> + where + F: Fn(&'p B) -> BoxFuture<'p, Result<()>>, + { + self.try_all_inner(what, node_effect, None, false, call) + .await + } + + async fn try_all_inner<'p, F>( + &'p self, + what: &'static str, + node_effect: &'static str, + all_failed: Option<&'static str>, + warn_per_node: bool, + call: F, + ) -> Result<()> where F: Fn(&'p B) -> BoxFuture<'p, Result<()>>, { @@ -179,7 +212,11 @@ impl FallbackBeaconNode { match call(node).await { Ok(()) => accepted += 1, Err(err) => { - warn!(%what, position, %err, node_effect, "Beacon node did not accept"); + if warn_per_node { + warn!(%what, position, %err, node_effect, "Beacon node did not accept"); + } else { + debug!(%what, position, %err, node_effect, "Beacon node did not accept"); + } last = Some(err); } } @@ -385,11 +422,11 @@ impl BeaconNodeApi for FallbackBeaconNode { fork: ForkName, preferences: &[gloas::SignedProposerPreferences], ) -> Result<()> { - self.try_all( + // Quiet: the service retries a refused preference every slot and warns + // once per entry, with what it costs. + self.try_all_quiet( "submit_proposer_preferences", "it will not gossip these proposer preferences", - // The service reports it, per preference, with what it costs. - None, |node| node.submit_proposer_preferences(fork, preferences), ) .await diff --git a/crates/validator/src/lib.rs b/crates/validator/src/lib.rs index 3eb4e19e4..65446736c 100644 --- a/crates/validator/src/lib.rs +++ b/crates/validator/src/lib.rs @@ -355,13 +355,6 @@ pub async fn run(config: ValidatorConfig) -> Result<()> { } } - // First in the slot, ahead of the proposal: the preferences for the - // slots after this one must be out before those slots start (gossip - // IGNOREs a started slot), and a poll queued behind a duty could land - // late. Off the slot loop, since it is single-flight and a slow node - // must not delay this slot's duties. - tokio::spawn(preferences.clone().poll(slot, store.clone())); - // A block is due now, at the boundary this iteration woke on. // // Cloned out of `duties` so the borrow ends before the await: the duty @@ -371,6 +364,15 @@ pub async fn run(config: ValidatorConfig) -> Result<()> { propose(&proposal, &clock, slot, &duty, &store).await; } + // Right after the proposal and ahead of the slot's other duties: the + // preferences for the slots after this one must be out before those + // slots start (gossip IGNOREs a started slot), and the poll's v2 duties + // fetch for the next epoch can make the node advance a state, which + // should not compete with it producing this slot's block. Off the slot + // loop, since it is single-flight and a slow node must not delay this + // slot's duties. + tokio::spawn(preferences.clone().poll(slot, store.clone())); + // The rest of the way to the first of the slot's remaining duties: // the attester offset, or the sync committee offset when the network // puts it earlier. Zero if the refresh or the proposal above already diff --git a/crates/validator/src/metrics.rs b/crates/validator/src/metrics.rs index 4367cedb3..16c8539bb 100644 --- a/crates/validator/src/metrics.rs +++ b/crates/validator/src/metrics.rs @@ -145,13 +145,24 @@ static PROPOSER_PREFERENCES_PUBLISHED_TOTAL: LazyLock = LazyLock::ne }); /// Proposer preferences no beacon node accepted, one count per preference per -/// attempt, plus failed v2 proposer-duty fetches. The next slot retries, so -/// this is a warning sign: until one is accepted, builders' bids for the slot -/// are ignored. +/// attempt. The next slot retries, so this is a warning sign: until one is +/// accepted, builders' bids for the slot are ignored. static PROPOSER_PREFERENCES_FAILURES_TOTAL: LazyLock = LazyLock::new(|| { register_int_counter!( "ethlambda_validator_proposer_preferences_failures_total", - "Proposer preferences no beacon node accepted, plus failed v2 duty fetches" + "Proposer preferences no beacon node accepted" + ) + .unwrap() +}); + +/// v2 proposer-duty fetches no beacon node answered, one count per target epoch +/// per attempt. Without the duties no preference is signed for that epoch, so a +/// node that is syncing or lacks the endpoint shows up here and not as refused +/// preferences. +static PROPOSER_PREFERENCES_DUTY_FETCH_FAILURES_TOTAL: LazyLock = LazyLock::new(|| { + register_int_counter!( + "ethlambda_validator_proposer_preferences_duty_fetch_failures_total", + "v2 proposer-duty fetches no beacon node answered" ) .unwrap() }); @@ -382,6 +393,7 @@ pub fn init() { LazyLock::force(&BUILDER_REGISTRATION_FAILURES_TOTAL); LazyLock::force(&PROPOSER_PREFERENCES_PUBLISHED_TOTAL); LazyLock::force(&PROPOSER_PREFERENCES_FAILURES_TOTAL); + LazyLock::force(&PROPOSER_PREFERENCES_DUTY_FETCH_FAILURES_TOTAL); LazyLock::force(&BLINDED_BLOCKS_PROPOSED_TOTAL); LazyLock::force(&BLINDED_BLOCKS_UNCONFIRMED_TOTAL); LazyLock::force(&BLOCKS_REFUSED_TOTAL); @@ -460,6 +472,10 @@ pub fn inc_proposer_preferences_failures() { PROPOSER_PREFERENCES_FAILURES_TOTAL.inc(); } +pub fn inc_proposer_preferences_duty_fetch_failures() { + PROPOSER_PREFERENCES_DUTY_FETCH_FAILURES_TOTAL.inc(); +} + pub fn inc_blinded_blocks_proposed() { BLINDED_BLOCKS_PROPOSED_TOTAL.inc(); } diff --git a/crates/validator/src/preferences.rs b/crates/validator/src/preferences.rs index 72ec3b501..8cb049a07 100644 --- a/crates/validator/src/preferences.rs +++ b/crates/validator/src/preferences.rs @@ -184,7 +184,7 @@ impl PreferencesService { Ok(duties) => duties, Err(err) => { debug!(epoch = target, %err, "Could not fetch proposer duties for preferences"); - crate::metrics::inc_proposer_preferences_failures(); + crate::metrics::inc_proposer_preferences_duty_fetch_failures(); if lock(&self.fetch_warned).insert(target) { warn!( epoch = target, @@ -641,6 +641,13 @@ mod tests { Root::repeat_byte(0x11), vec![duty(&pubkeys[0], 3, slot_in(GLOAS_EPOCH, 10))], ); + // Both target epochs have duties, and the failure is injected on the + // call itself, so neither fetch fails for want of data. + node.set_proposers_v2( + GLOAS_EPOCH + 1, + Root::repeat_byte(0x22), + vec![duty(&pubkeys[0], 3, slot_in(GLOAS_EPOCH + 1, 4))], + ); node.failures .lock() .expect("lock") @@ -651,12 +658,23 @@ mod tests { service.clone().poll(current, store.clone()).await; service.clone().poll(current + 1, store.clone()).await; assert!(submitted(&node).is_empty()); - let warned: Vec = lock(&service.fetch_warned).iter().copied().collect(); - assert_eq!(warned.len(), 2, "one entry per target epoch, not per poll"); + assert_eq!( + *lock(&service.fetch_warned), + HashSet::from([GLOAS_EPOCH, GLOAS_EPOCH + 1]), + "one entry per target epoch, not per poll" + ); node.failures.lock().expect("lock").clear(); service.poll(current + 2, store).await; - assert_eq!(submitted(&node).len(), 1); + let mut slots: Vec = submitted(&node) + .iter() + .flat_map(|(_, entries)| entries.iter().map(|entry| entry.message.proposal_slot)) + .collect(); + slots.sort(); + assert_eq!( + slots, + vec![slot_in(GLOAS_EPOCH, 10), slot_in(GLOAS_EPOCH + 1, 4)] + ); } #[tokio::test] diff --git a/docs/metrics.md b/docs/metrics.md index 1788fd5a3..2ed3f79ef 100644 --- a/docs/metrics.md +++ b/docs/metrics.md @@ -744,7 +744,8 @@ and absent is not the same as zero. | `ethlambda_validator_builder_registrations_sent_total` | Counter | Builder registrations a beacon node accepted (`--builder-proposals`) | | `ethlambda_validator_builder_registration_failures_total` | Counter | Registration batches no beacon node accepted. A missed registration costs at most a builder block, never a duty | | `ethlambda_validator_proposer_preferences_published_total` | Counter | Signed proposer preferences a beacon node accepted, counted per preference. Published for every proposal from the epoch before gloas, whether or not `--builder-proposals` is set | -| `ethlambda_validator_proposer_preferences_failures_total` | Counter | Proposer preferences a beacon node refused or failed to take (one count per preference per attempt), plus failed v2 proposer-duty fetches. The next slot retries; until a preference is accepted, builders' bids for its slot are ignored | +| `ethlambda_validator_proposer_preferences_failures_total` | Counter | Proposer preferences a beacon node refused or failed to take (one count per preference per attempt). The next slot retries; until a preference is accepted, builders' bids for its slot are ignored | +| `ethlambda_validator_proposer_preferences_duty_fetch_failures_total` | Counter | v2 proposer-duty fetches no beacon node answered (one count per target epoch per attempt). Without the duties no preference is signed for that epoch; a syncing node, or one without the v2 endpoint, shows up here | | `ethlambda_validator_blinded_blocks_proposed_total` | Counter | Blinded blocks a beacon node handed to its builder. A subset of `blocks_proposed_total` | | `ethlambda_validator_blinded_blocks_unconfirmed_total` | Counter | Blinded blocks the beacon node did not confirm (a 5xx or no answer), which the relay may still have published. Not retried, since a second block for the slot would be an equivocation | | `ethlambda_validator_block_publication_delay_seconds` | Histogram | Slot start to block accepted. Its buckets are tighter than the attestation histogram's, because a block is due at the slot boundary rather than a third of the way in | From 34a914f812d24c4af0e912592dea1ea900948b29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Thu, 29 Oct 2026 17:49:31 -0300 Subject: [PATCH 10/16] docs: carry the keymanager rows and the README duties into gloas From gloas the proposer preferences carry each validator's fee recipient and target gas limit, so the keymanager's `feerecipient` and `gas_limit` reach the node through them, from the next dependent root on, whether or not `--builder-proposals` is set. The README's duty list, written on main by #716, gains the preferences, which are published every slot. --- README.md | 2 +- docs/cli.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index e111d22f4..af265a424 100644 --- a/README.md +++ b/README.md @@ -338,7 +338,7 @@ endpoints. A separate process that holds the keys and performs the duties: attestations, block proposals, aggregation, sync committee messages and, from Gloas on, the -payload envelope and the payload timeliness vote. It talks to a beacon node over +payload envelope, proposer preferences and the payload timeliness vote. It talks to a beacon node over the standard Beacon API, so it works against `ethlambda beacon` (run with the `--execution-*` flags) or any other conformant beacon node. diff --git a/docs/cli.md b/docs/cli.md index 1e387769f..ac359877b 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -448,9 +448,9 @@ give one validator its own value in place of the command line's default: | Setting | Default | Takes effect | |---|---|---| -| `feerecipient` | `--suggested-fee-recipient` | At the next epoch boundary, when the fee recipients are registered with the beacon node again. A block proposed before then pays the address the node was last given, and the check before signing reports the mismatch. With `--builder-proposals`, the next slot's builder registration carries it too, and an ethlambda beacon node takes it from there | +| `feerecipient` | `--suggested-fee-recipient` | At the next epoch boundary, when the fee recipients are registered with the beacon node again. A block proposed before then pays the address the node was last given, and the check before signing reports the mismatch. With `--builder-proposals`, the next slot's builder registration carries it too, and an ethlambda beacon node takes it from there. From gloas, the proposer preferences carry it, from the next dependent root on | | `graffiti` | `--graffiti` | The next proposal | -| `gas_limit` | `DEFAULT_GAS_LIMIT` | With `--builder-proposals`, the next slot's builder registration, which the builder builds toward. Otherwise stored and served only | +| `gas_limit` | `DEFAULT_GAS_LIMIT` | Before gloas, the next slot's builder registration with `--builder-proposals`, and stored and served only without it. From gloas, the proposer preferences' target gas limit, from the next dependent root on, flag or not | **These are held in memory only.** A restart puts every validator back on the defaults, so an operator driving these endpoints must set them again after one. From 68cb7e08740496495e79fba4912ceb2f5cc42d0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Thu, 5 Nov 2026 18:26:52 -0300 Subject: [PATCH 11/16] fix(validator): sign proposer preferences at the proposal epoch's fork version consensus-specs v1.7.0-beta.4 signs (`get_signed_proposer_preferences`) and verifies (`validate_proposer_preferences_gossip`) proposer preferences with `compute_fork_version(proposal_epoch)`, and Lighthouse, Prysm, Teku, Lodestar and Nimbus all do the same. The validator client still signed by beta.2's gossip rule, the fork of the epoch before the proposal's, which differs only in the first epoch of a fork; on a devnet Lighthouse refused every one of its first-gloas-epoch preferences as a bad signature. - Signer: `SigningContext::sign_proposer_preferences` now uses the proposal epoch's fork. Its tests assert gloas's version and not fulu's for the first and the second gloas epoch. - Node: `proposer_preferences_domains` still accepts all three candidates, but tries the proposal epoch's first; beta.2's two are kept until the fixtures (pinned to beta.2) move past it. - New end-to-end test with fulu before gloas: the validator client's preference for the first gloas epoch passes the node's gossip rules and verifies under gloas's version, not fulu's. - The route comment and docs no longer say a validator client names fulu, or that the client signs by the gossip rule's epoch-before domain. Claude-Session: https://claude.ai/code/session_011vghNneoZ1Ta9W6JNqzjxm --- CLAUDE.md | 5 +- .../src/beacon/gossip/proposer_preferences.rs | 38 ++++-- .../rpc/src/beacon/builder_market_tests.rs | 122 ++++++++++++++---- .../rpc/src/beacon/proposer_preferences.rs | 6 +- crates/validator/src/preferences.rs | 24 ++-- crates/validator/src/signing.rs | 119 +++++++++-------- docs/rpc.md | 12 +- docs/spec_deviations.md | 45 +++---- 8 files changed, 224 insertions(+), 147 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 0b14398a0..38f0a3bce 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -866,8 +866,9 @@ transitions are in `ethlambda-types`, per the section above. Nothing above `--builder-proposals`, pre-gloas slots register validators with the node's builder and may sign a blinded block, published to the producing node only. From the epoch before gloas it signs proposer preferences every slot for its proposals in - the current and next epoch (`PreferencesService`, v2 duties, the gossip rule's - signing domain), unconditionally, and `produceBlockV4` carries the boost factor. + the current and next epoch (`PreferencesService`, v2 duties, signed at the + proposal epoch's fork version as consensus-specs v1.7.0-beta.4 and every other + client do), unconditionally, and `produceBlockV4` carries the boost factor. It still keeps no slashing-protection record; PTC votes have an in-memory `(validator, slot)` dedup only, and envelopes are unguarded. `get_head_node` is one bottom-up walk for every fork, with diff --git a/crates/blockchain/state_transition/src/beacon/gossip/proposer_preferences.rs b/crates/blockchain/state_transition/src/beacon/gossip/proposer_preferences.rs index f278ea62e..e3735bae5 100644 --- a/crates/blockchain/state_transition/src/beacon/gossip/proposer_preferences.rs +++ b/crates/blockchain/state_transition/src/beacon/gossip/proposer_preferences.rs @@ -16,8 +16,10 @@ //! dependent root (the whole canonical case, and a dependent block about an //! epoch old is usually evicted from the state cache), else from the cached //! checkpoint state of the epoch before the proposal's. -//! - The signature verifies under any of three domains, since clients disagree -//! at the fork boundary: see [`proposer_preferences_domains`]. +//! - The signature verifies under the proposal epoch's domain (v1.7.0-beta.4's +//! rule, and every client's) or under either of the two that beta.2's rule +//! could give, which differ from it only in the first epoch of a fork: see +//! [`proposer_preferences_domains`]. use ethlambda_storage::CacheKey; @@ -208,15 +210,22 @@ pub fn dependent_root_at( ancestor_at(state, state_block_root, dependent_slot) } -/// The distinct candidate signing domains, tried in order: `get_domain( -/// lookahead_state, DOMAIN_PROPOSER_PREFERENCES, Some(P))` (the specification's); -/// the schedule's fork version at `P - MIN_SEED_LOOKAHEAD` (saturating); and the -/// schedule's at `P`. +/// The distinct candidate signing domains for proposal epoch `P`, tried in +/// order. /// -/// Outside the first epoch of a fork the three coincide. Across a boundary -/// the specification's gives the version of the epoch before the proposal's, -/// while lighthouse signs with the proposal epoch's, so accepting both avoids -/// rejecting an honest client's messages around the gloas upgrade. +/// First the schedule's fork version at `P`: consensus-specs v1.7.0-beta.4's +/// `compute_fork_version(proposal_epoch)`, which `get_signed_proposer_preferences` +/// signs with and `validate_proposer_preferences_gossip` verifies with, and +/// what every client (this one's validator client included) signs under. +/// +/// Then the two that beta.2's rule, `get_domain(lookahead_state, +/// DOMAIN_PROPOSER_PREFERENCES, P)`, could give: `get_domain` on the state +/// this node read the lookahead from, and the schedule's version at +/// `P - MIN_SEED_LOOKAHEAD` (saturating), the epoch beta.2 advances its state +/// to (this node may have read the head's, a later one). They are kept for a +/// client still signing by beta.2, until the fixtures move past it. Outside +/// the first epoch of a fork all three coincide, so accepting the older ones +/// costs nothing. pub fn proposer_preferences_domains( lookahead_state: &BeaconState, config: &Config, @@ -231,13 +240,13 @@ pub fn proposer_preferences_domains( ) }; let domains = [ + at(proposal_epoch), get_domain( lookahead_state, DOMAIN_PROPOSER_PREFERENCES, Some(proposal_epoch), ), at(proposal_epoch.saturating_sub(preset::MIN_SEED_LOOKAHEAD)), - at(proposal_epoch), ]; let mut distinct: Vec = Vec::with_capacity(domains.len()); for domain in domains { @@ -595,10 +604,11 @@ mod tests { let root = state.genesis_validators_root(); let at = |fork| compute_domain(DOMAIN_PROPOSER_PREFERENCES, config.fork_version(fork), root); - // Proposal epoch 2 is gloas's first: the lookahead state's fork (fulu) - // and the epoch before the proposal's agree, the proposal epoch's is gloas. + // Proposal epoch 2 is gloas's first. The proposal epoch's version + // (gloas, beta.4's rule) comes first; the lookahead state's fork and + // the epoch before the proposal's (beta.2's) agree on fulu. let domains = proposer_preferences_domains(&state, &config, 2); - assert_eq!(domains, vec![at(ForkName::Fulu), at(ForkName::Gloas)]); + assert_eq!(domains, vec![at(ForkName::Gloas), at(ForkName::Fulu)]); // A signature under either verifies; one under neither does not. let preferences = gloas::ProposerPreferences { diff --git a/crates/net/rpc/src/beacon/builder_market_tests.rs b/crates/net/rpc/src/beacon/builder_market_tests.rs index 2db925ce6..bb0004f54 100644 --- a/crates/net/rpc/src/beacon/builder_market_tests.rs +++ b/crates/net/rpc/src/beacon/builder_market_tests.rs @@ -18,6 +18,7 @@ use ethlambda_engine::{EngineClient, JwtSecret}; use ethlambda_network_api::RpcToP2PRef; use ethlambda_state_transition::beacon::{ block_production::advance_to_slot, + bls, builder_market::SharedBuilderMarket, gloas_block_production::test_support::{ config as chain_config, parent_state, post_state, randao_reveal, @@ -25,7 +26,7 @@ use ethlambda_state_transition::beacon::{ gossip::proposer_preferences::dependent_root_at, helpers::{ accessors::get_domain, - misc::compute_signing_root, + misc::{compute_domain, compute_signing_root}, test_state::{secret_key_for, sign_for}, }, payload_attestation_pool::SharedPayloadAttestationPool, @@ -35,18 +36,20 @@ use ethlambda_state_transition::beacon::{ use ethlambda_storage::Store; use ethlambda_types::{ beacon::{ + config::Config, constants::{ BUILDER_INDEX_SELF_BUILD, DOMAIN_BEACON_BUILDER, DOMAIN_PROPOSER_PREFERENCES, FAR_FUTURE_EPOCH, PAYLOAD_BUILDER_VERSION, }, containers::{ - BeaconState, SignedBeaconBlock, + BeaconState, Fork, SignedBeaconBlock, gloas::{ BeaconBlock, Builder, ExecutionPayloadBid, ExecutionRequests, ProposerPreferences, SignedExecutionPayloadBid, SignedExecutionPayloadEnvelope, SignedProposerPreferences, }, }, + fork::ForkName, primitives::{BlsPubkey, BlsSignature, ExecutionAddress, HashTreeRoot as _}, signing::compute_epoch_at_slot, }, @@ -217,6 +220,8 @@ fn chain_state() -> BeaconState { } struct World { + /// The fork schedule the store runs on. + config: Config, store: Store, state: BeaconState, head_root: H256, @@ -240,7 +245,33 @@ impl World { } fn at_clock(clock_slot: u64) -> Self { - let state = chain_state(); + Self::build(chain_config(), chain_state(), clock_slot) + } + + /// [`World::new`] with fulu before gloas: gloas starts at epoch 1, the one + /// [`SLOT`] and the head are in, so a preference for [`SLOT`] is for the + /// first gloas epoch. The head state carries the fork record + /// `upgrade_to_gloas` leaves at that boundary. + fn at_the_gloas_fork() -> Self { + let config = ForkName::ALL + .into_iter() + .filter(|&fork| fork < ForkName::Gloas) + .fold(chain_config(), |config, fork| { + config.with_fork_epoch(fork, 0) + }) + .with_fork_epoch(ForkName::Gloas, compute_epoch_at_slot(SLOT)); + let mut state = chain_state(); + *state.fork_mut() = Fork { + previous_version: config.fork_version(ForkName::Fulu), + current_version: config.fork_version(ForkName::Gloas), + epoch: compute_epoch_at_slot(SLOT), + }; + Self::build(config, state, HEAD_SLOT) + } + + /// The head block on `state` and `config`, on a clock where `clock_slot` + /// is running. + fn build(config: Config, state: BeaconState, clock_slot: u64) -> Self { let genesis = genesis_block(); let genesis_root = genesis.message_hash_tree_root(); let block = gloas_beacon_block( @@ -255,7 +286,7 @@ impl World { let head_root = header.hash_tree_root(); let mut store = beacon_store_with_head_block( state.clone(), - chain_config(), + config.clone(), block, head_root, clock_slot, @@ -263,6 +294,7 @@ impl World { store.insert_signed_block(genesis_root, genesis).unwrap(); store.insert_state(genesis_root, state.clone()).unwrap(); Self { + config, store, state, head_root, @@ -296,7 +328,7 @@ impl World { } fn advanced(&self) -> BeaconState { - advance_to_slot(&self.state, SLOT, &chain_config()).unwrap() + advance_to_slot(&self.state, SLOT, &self.config).unwrap() } /// What `produceBlockV4` reads off the chain for [`SLOT`]. @@ -1084,18 +1116,15 @@ async fn valid_preferences_are_cached_and_published_once() { assert_eq!(world.network.proposer_preferences.lock().unwrap().len(), 1); } -/// The validator client's own path end to end against this node: it reads the -/// proposer duty and its v2 `dependent_root` from the served duties route, -/// signs with `SigningContext::sign_proposer_preferences`, and posts through -/// `HttpBeaconNode`, and the node's gossip rules accept it. That pins the -/// duties route's dependent root as the one the preference must carry, and the -/// JSON the client posts as the one the route decodes. -/// -/// It does not pin the fork-version choice: this world has gloas at epoch 0 -/// and the node accepts either version, so `signing.rs`'s unit test is the pin -/// for the domain at a fork boundary. -#[tokio::test] -async fn preferences_signed_by_the_validator_client_pass_this_nodes_gossip_rules() { +/// The validator client's own path for a proposal at [`SLOT`], end to end +/// against `world`'s node: it reads the proposer duty and its v2 +/// `dependent_root` from the served duties route, signs with +/// `SigningContext::sign_proposer_preferences`, and posts through +/// `HttpBeaconNode` under the fork of the preference's slot, as +/// `PreferencesService` does. The post succeeding means the route's run of the +/// gossip rules (`validate`) accepted it; this also checks that the market +/// holds it and that it went out. Returns it with the proposer's key. +async fn submit_as_the_validator_client(world: &World) -> (SignedProposerPreferences, BlsPubkey) { use ethlambda_blockchain::{SyncStatusController, metrics::SyncStatus}; use ethlambda_validator::{ beacon_node::{BeaconNodeApi as _, dto::parse_pubkey, http::HttpBeaconNode}, @@ -1103,7 +1132,6 @@ async fn preferences_signed_by_the_validator_client_pass_this_nodes_gossip_rules signing::SigningContext, }; - let world = World::new(); let duties_routes = crate::beacon::validator::routes() .with_state(world.store.clone()) .layer(Extension(SyncStatusController::new(SyncStatus::Synced))); @@ -1114,8 +1142,9 @@ async fn preferences_signed_by_the_validator_client_pass_this_nodes_gossip_rules let client = HttpBeaconNode::new(format!("http://{bound}")).unwrap(); // The duty and the dependent root the way the validator client reads them. + let proposal_epoch = compute_epoch_at_slot(SLOT); let duties = client - .proposer_duties_v2(compute_epoch_at_slot(SLOT)) + .proposer_duties_v2(proposal_epoch) .await .expect("the node serves v2 proposer duties"); let duty = duties @@ -1142,7 +1171,7 @@ async fn preferences_signed_by_the_validator_client_pass_this_nodes_gossip_rules "the test registry's key for the proposer" ); let context = SigningContext { - config: chain_config(), + config: world.config.clone(), genesis_validators_root: world.state.genesis_validators_root(), }; let signature = context @@ -1150,11 +1179,9 @@ async fn preferences_signed_by_the_validator_client_pass_this_nodes_gossip_rules .expect("signs"); let signed = SignedProposerPreferences { message, signature }; + let fork = world.config.fork_at_epoch(proposal_epoch); client - .submit_proposer_preferences( - ethlambda_types::beacon::fork::ForkName::Gloas, - std::slice::from_ref(&signed), - ) + .submit_proposer_preferences(fork, std::slice::from_ref(&signed)) .await .expect("the node accepts what the validator client signed"); @@ -1166,8 +1193,53 @@ async fn preferences_signed_by_the_validator_client_pass_this_nodes_gossip_rules world .market .preferences(SLOT, signed.message.dependent_root), - Some(signed) + Some(signed.clone()) + ); + (signed, pubkey) +} + +/// The validator client's preference passes this node's gossip rules. That +/// pins the duties route's dependent root as the one the preference must +/// carry, and the JSON the client posts as the one the route decodes. This +/// world has gloas at epoch 0, so no fork boundary is involved; the next test +/// has one. +#[tokio::test] +async fn preferences_signed_by_the_validator_client_pass_this_nodes_gossip_rules() { + submit_as_the_validator_client(&World::new()).await; +} + +/// The same across a fulu-to-gloas boundary, for a slot in the first gloas +/// epoch: the one case where the fork of the proposal's epoch (gloas, what +/// consensus-specs v1.7.0-beta.4 and every client sign and verify with) and +/// the fork of the epoch before it (fulu, beta.2's gossip rule) differ. +/// +/// This node accepts both, so its verdict alone would also pass a client still +/// signing by beta.2. The signature is therefore also checked against the two +/// domains directly: it must verify under gloas's version, which Lighthouse +/// and the others demand, and not under fulu's, which Lighthouse refused. +#[tokio::test] +async fn a_first_gloas_epoch_preference_from_the_validator_client_passes_this_nodes_gossip_rules() { + let world = World::at_the_gloas_fork(); + let proposal_epoch = compute_epoch_at_slot(SLOT); + assert_eq!( + world.config.fork_at_epoch(proposal_epoch - 1), + ForkName::Fulu ); + assert_eq!(world.config.fork_at_epoch(proposal_epoch), ForkName::Gloas); + + let (signed, pubkey) = submit_as_the_validator_client(&world).await; + + let verifies_under = |fork| { + let domain = compute_domain( + DOMAIN_PROPOSER_PREFERENCES, + world.config.fork_version(fork), + world.state.genesis_validators_root(), + ); + let root = compute_signing_root(signed.message.hash_tree_root(), domain); + bls::verify(&pubkey, root, &signed.signature) + }; + assert!(verifies_under(ForkName::Gloas), "the proposal epoch's fork"); + assert!(!verifies_under(ForkName::Fulu), "not the epoch before it"); } #[tokio::test] diff --git a/crates/net/rpc/src/beacon/proposer_preferences.rs b/crates/net/rpc/src/beacon/proposer_preferences.rs index bd217f0b5..264576e8e 100644 --- a/crates/net/rpc/src/beacon/proposer_preferences.rs +++ b/crates/net/rpc/src/beacon/proposer_preferences.rs @@ -65,8 +65,10 @@ async fn post_proposer_preferences( headers: HeaderMap, body: Bytes, ) -> Response { - // A validator client submits during the epoch before gloas, when the - // consensus version it names is still fulu. + // Every validator client names gloas, even during the epoch before it: + // Prysm hardcodes it, the others (this one's too) take the fork of the + // preference's slot. Fulu is still accepted, as leniency towards a client + // that names the fork of its wall clock instead. if let Err(err) = require_version(&headers, &[ForkName::Fulu, ForkName::Gloas]) { return err.into_response(); } diff --git a/crates/validator/src/preferences.rs b/crates/validator/src/preferences.rs index 8cb049a07..a72b19c28 100644 --- a/crates/validator/src/preferences.rs +++ b/crates/validator/src/preferences.rs @@ -21,10 +21,11 @@ //! //! # The signing domain //! -//! See [`SigningContext::sign_proposer_preferences`]: the gossip rule's domain, -//! at the fork of the epoch before the proposal's. The validator guide's -//! head-state signer differs from it only for a proposal in the first epoch of -//! a fork, signed during that epoch. +//! See [`SigningContext::sign_proposer_preferences`]: the fork version of the +//! proposal epoch itself, as consensus-specs v1.7.0-beta.4 signs and verifies +//! and as every other client does. Beta.2's gossip rule took the fork of the +//! epoch before the proposal's, which differs only for a proposal in the first +//! epoch of a fork, and Lighthouse refuses a preference signed that way. //! //! # Why every slot //! @@ -374,18 +375,13 @@ mod tests { let context = context(); let proposal_epoch = compute_epoch_at_slot(entry.message.proposal_slot); - // Spelled out rather than asked of `SigningContext::domain`: the - // first gloas epoch's preference verifies under fulu's version (the - // fork of the epoch before it), every later one under gloas's. - let version = if proposal_epoch == GLOAS_EPOCH { - context.config.fulu_fork_version - } else { - assert!(proposal_epoch > GLOAS_EPOCH); - context.config.gloas_fork_version - }; + // Spelled out rather than asked of `SigningContext::domain`: every + // preference, the first gloas epoch's included, verifies under the + // proposal epoch's version, which for every slot here is gloas's. + assert!(proposal_epoch >= GLOAS_EPOCH); let domain = compute_domain( DOMAIN_PROPOSER_PREFERENCES, - version, + context.config.gloas_fork_version, context.genesis_validators_root, ); let root = compute_signing_root(entry.message.hash_tree_root(), domain); diff --git a/crates/validator/src/signing.rs b/crates/validator/src/signing.rs index 2ece1aca3..c63c4b730 100644 --- a/crates/validator/src/signing.rs +++ b/crates/validator/src/signing.rs @@ -26,7 +26,6 @@ use ethlambda_types::beacon::primitives::{ }; // `AttestationData`, `Checkpoint`, `Fork` and `SigningData` all live in // `containers::shared`, verified against `containers/shared.rs:158,120,97,220`. -use ethlambda_types::beacon::preset; use ethlambda_types::beacon::signing::{ compute_builder_domain, compute_domain, compute_epoch_at_slot, compute_signing_root, }; @@ -319,19 +318,22 @@ impl SigningContext { } /// A proposer preferences message's signature, under - /// `DOMAIN_PROPOSER_PREFERENCES` at the fork version of the epoch before - /// the proposal's. + /// `DOMAIN_PROPOSER_PREFERENCES` at the fork version of the proposal + /// epoch itself. /// - /// That is the domain the `proposer_preferences` gossip rule verifies - /// against: `get_domain(state, .., proposal_epoch)` on the dependent - /// block's state advanced to the lookahead start, whose fork is the one - /// in force an epoch before the proposal. The validator guide signs with - /// the head state (`get_domain` reads `state.fork`, not the schedule); - /// the two agree everywhere except for a proposal in the first epoch of a - /// fork, signed during that epoch: the head state already carries the new - /// version, while the gossip rule's state, at the start of the previous - /// epoch, still carries the old one. This signer takes the gossip rule's, - /// which is the one peers verify. Not slashable. + /// That is `get_signed_proposer_preferences` as of consensus-specs + /// v1.7.0-beta.4, `compute_fork_version(proposal_epoch)` read off the + /// schedule, and the `proposer_preferences` gossip rule verifies under the + /// same domain. Every other client signs and verifies this way + /// (Lighthouse, Prysm, Teku, Lodestar and Nimbus all take the fork of the + /// proposal's epoch). + /// + /// The rule before it, beta.2's, verified with `get_domain` on the + /// dependent block's state advanced to the lookahead start, which gives + /// the fork of the epoch before the proposal's. That differs only for a + /// proposal in the first epoch of a fork, and there Lighthouse refuses it + /// as a bad signature: it did so for every first-gloas-epoch preference + /// this client signed that way on a devnet. Not slashable. pub fn sign_proposer_preferences( &self, store: &ValidatorStore, @@ -339,10 +341,7 @@ impl SigningContext { preferences: &ProposerPreferences, ) -> Result { let proposal_epoch = compute_epoch_at_slot(preferences.proposal_slot); - let domain = self.domain( - DOMAIN_PROPOSER_PREFERENCES, - proposal_epoch.saturating_sub(preset::MIN_SEED_LOOKAHEAD), - ); + let domain = self.domain(DOMAIN_PROPOSER_PREFERENCES, proposal_epoch); let root = compute_signing_root(preferences.hash_tree_root(), domain); self.sign_root(store, pubkey, root) } @@ -646,63 +645,61 @@ mod tests { } } - /// The first gloas epoch's preference is signed at the fork of the epoch - /// before it (fulu), which is what the gossip rule verifies against. The - /// validator guide's head-state signer would give gloas for a preference - /// signed during that epoch, which the gossip rule's state (still fulu) - /// would reject. - #[test] - fn a_preference_for_the_first_gloas_epoch_is_signed_under_the_fulu_version() { + /// Whether a preference for `slot`, signed by the client, verifies under + /// fulu's and under gloas's version of `DOMAIN_PROPOSER_PREFERENCES`, in + /// that order. The domains are spelled out rather than asked of + /// [`SigningContext::domain`], so the test does not share the signer's + /// choice of epoch. + fn preference_verifies_under_fulu_and_gloas(slot: Slot) -> (bool, bool) { let (store, pubkey) = store_with_key(); - let (context, gloas_epoch) = gloas_scheduled_context(); - let preferences = preferences_for(gloas_epoch * preset::SLOTS_PER_EPOCH + 2); + let (context, _) = gloas_scheduled_context(); + let preferences = preferences_for(slot); let signature = context .sign_proposer_preferences(&store, &pubkey, &preferences) .expect("signs"); - let under = |version| { - compute_signing_root( - preferences.hash_tree_root(), - compute_domain( - DOMAIN_PROPOSER_PREFERENCES, - version, - context.genesis_validators_root, - ), - ) + let verifies_under = |version| { + let domain = compute_domain( + DOMAIN_PROPOSER_PREFERENCES, + version, + context.genesis_validators_root, + ); + let root = compute_signing_root(preferences.hash_tree_root(), domain); + verify(&pubkey, &signature, root) }; - assert!(verify( - &pubkey, - &signature, - under(context.config.fulu_fork_version) - )); - assert!(!verify( - &pubkey, - &signature, - under(context.config.gloas_fork_version) - )); + ( + verifies_under(context.config.fulu_fork_version), + verifies_under(context.config.gloas_fork_version), + ) + } + + /// The first gloas epoch's preference is signed at gloas's version, the + /// proposal epoch's own, which is what beta.4's gossip rule and every + /// other client verify. Fulu's, the version beta.2's rule gave (the epoch + /// before the proposal's), is what Lighthouse refused as a bad signature. + #[test] + fn a_preference_for_the_first_gloas_epoch_is_signed_under_the_gloas_version() { + let (_, gloas_epoch) = gloas_scheduled_context(); + let slot = gloas_epoch * preset::SLOTS_PER_EPOCH + 2; + + let (under_fulu, under_gloas) = preference_verifies_under_fulu_and_gloas(slot); + + assert!(under_gloas, "the proposal epoch's version"); + assert!(!under_fulu, "not the version of the epoch before it"); } - /// One epoch later the epoch before the proposal's is itself gloas. + /// One epoch later beta.2's rule gives gloas's version too, so the two + /// rules agree from here on. #[test] fn a_preference_for_the_second_gloas_epoch_is_signed_under_the_gloas_version() { - let (store, pubkey) = store_with_key(); - let (context, gloas_epoch) = gloas_scheduled_context(); - let preferences = preferences_for((gloas_epoch + 1) * preset::SLOTS_PER_EPOCH); + let (_, gloas_epoch) = gloas_scheduled_context(); + let slot = (gloas_epoch + 1) * preset::SLOTS_PER_EPOCH; - let signature = context - .sign_proposer_preferences(&store, &pubkey, &preferences) - .expect("signs"); + let (under_fulu, under_gloas) = preference_verifies_under_fulu_and_gloas(slot); - let root = compute_signing_root( - preferences.hash_tree_root(), - compute_domain( - DOMAIN_PROPOSER_PREFERENCES, - context.config.gloas_fork_version, - context.genesis_validators_root, - ), - ); - assert!(verify(&pubkey, &signature, root)); + assert!(under_gloas); + assert!(!under_fulu); } #[test] diff --git a/docs/rpc.md b/docs/rpc.md index c6063c144..872f3707e 100644 --- a/docs/rpc.md +++ b/docs/rpc.md @@ -679,11 +679,13 @@ message accepted from either side is seen by both. - **`POST /eth/v1/validator/proposer_preferences`** takes a list of `SignedProposerPreferences` (at most `(MIN_SEED_LOOKAHEAD + 1) * SLOTS_PER_EPOCH`, 64 on mainnet), as a JSON array or an SSZ list. - `Eth-Consensus-Version` is optional and may be `fulu` or `gloas`, since a - validator client submits during the epoch before the fork. Each entry runs the - gossip rules in order; an entry already cached, identical, succeeds without a - republish. Accepted entries are cached (the first per `(proposal slot, - dependent root)` wins) and gossiped. If any entry fails the answer is `400` + `Eth-Consensus-Version` is optional and may be `fulu` or `gloas`. Validator + clients name `gloas` even during the epoch before the fork (the fork of the + preference's slot); `fulu` is accepted as leniency towards one that names its + wall clock's fork. Each entry runs the gossip rules in order; an entry already + cached, identical, succeeds without a republish. Accepted entries are cached + (the first per `(proposal slot, dependent root)` wins) and gossiped. If any + entry fails the answer is `400` `{code, message, failures: [{index, message}]}` and the others were still published. The endpoint is not gated on the sync status. A node whose dependent block is unknown answers `ignore: unknown_block` for that entry. diff --git a/docs/spec_deviations.md b/docs/spec_deviations.md index 3cc31e03b..765dcae1d 100644 --- a/docs/spec_deviations.md +++ b/docs/spec_deviations.md @@ -264,12 +264,14 @@ from the Beacon API, and no other way. and a bid is compatible with it when it builds on that head and its payload. The builder-exit check is skipped for a pre-gloas parent, which carries no envelope. -- **Preference signatures** are accepted under the lookahead state's own - `DOMAIN_PROPOSER_PREFERENCES` domain (the specification's), the fork version of - the epoch before the proposal's, or the proposal epoch's. They coincide outside - the first epoch of a fork. Lighthouse signs with the proposal epoch's version - and the specification gives the earlier one, so accepting both avoids - rejecting an honest client's messages around the gloas upgrade. +- **Preference signatures** are accepted under `DOMAIN_PROPOSER_PREFERENCES` at + the proposal epoch's fork version, which consensus-specs v1.7.0-beta.4 signs + and verifies with and every client signs with, and also under the two that + v1.7.0-beta.2's rule (`get_domain` on the lookahead state) could give: that + state's own fork, and the fork version of the epoch before the proposal's. + They coincide outside the first epoch of a fork. The older two are kept for a + client still signing by beta.2, until the fixtures (pinned to beta.2) move + past it. - **Consequence:** builders onboarded at the fork are inactive until their deposit epoch is finalized, so gossip bids are rejected for the first epochs of gloas and `produceBlockV4` self-builds. @@ -768,24 +770,19 @@ the beacon node's builder and may sign a blinded block (builder-specs and Validators with no fee recipient are not registered either (the field is required), and only validators the beacon node resolved are signed for. -## The validator client signs proposer preferences under the gossip rule's domain - -`validator.md` ("Broadcasting `SignedProposerPreferences`") signs with -`get_domain(state, DOMAIN_PROPOSER_PREFERENCES, compute_epoch_at_slot(proposal_slot))` -on the head state. `p2p-interface.md`'s `validate_proposer_preferences_gossip` -verifies against the dependent block's state advanced to the lookahead start, -so the fork version it uses is the one in force an epoch before the proposal's. - -- **ethlambda:** the validator client signs at the epoch before the proposal's - (`SigningContext::sign_proposer_preferences`), which is the domain the gossip - rule verifies. -- **Why:** `get_domain` reads `state.fork`, not the schedule, so the two agree - everywhere except for a proposal in the first epoch of a fork, signed during - that epoch. There the head state already carries the new fork version, while - the gossip rule's state (at the start of the previous epoch) still carries the - old one, so the guide's signature would fail the rule peers apply. During the - epoch before the fork both give the old version. This node's own - `proposer_preferences` endpoint accepts both. +## The validator client's proposer preferences + +- **Signing domain: no departure, but ahead of the fixtures.** The validator + client signs under `DOMAIN_PROPOSER_PREFERENCES` at the proposal epoch's fork + version (`SigningContext::sign_proposer_preferences`), as consensus-specs + v1.7.0-beta.4's `get_signed_proposer_preferences` does with + `compute_fork_version(proposal_epoch)`, and as Lighthouse, Prysm, Teku, + Lodestar and Nimbus all sign and verify. The fixtures are still pinned to + v1.7.0-beta.2, whose gossip rule verified with `get_domain` on the dependent + block's state advanced to the lookahead start, the fork of the epoch before + the proposal's. The two differ only for a proposal in the first epoch of a + fork, where Lighthouse refuses beta.2's signature as a bad one. This node + still accepts beta.2's domain too (see the fulu-to-gloas boundary entry). - **Published unconditionally,** not gated on `--builder-proposals`, since from gloas the preferences carry every proposal's fee recipient. Validators with no fee recipient are skipped (the field is required), and a preference cannot be From 7edb397ee0ab1aa603d38a1882d65f63e2294c1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Thu, 5 Nov 2026 18:33:33 -0300 Subject: [PATCH 12/16] fix(validator): bound preference duties and re-send until every node holds them Security review of the proposer preferences service: - Bound the v2 duties before signing: drop entries outside the target epoch and keep the first per slot, so a node's answer costs at most SLOTS_PER_EPOCH signatures, sends and remembered keys per epoch. - A preference is done only once every node accepted it. The fallback now answers Delivery::{Everywhere, Partly}; a partly delivered preference is kept as signed and re-sent to every node each slot, so a node that was syncing still gets it, and a fee recipient change in between cannot split the nodes. Counted and logged once, at the first acceptance. - Read the fee recipient and gas limit in one settings snapshot. - Use the shared InFlight guard, and fix InFlight::enter: then_some built the guard eagerly, so a refused enter dropped it and cleared the running run's flag. Claude-Session: https://claude.ai/code/session_011vghNneoZ1Ta9W6JNqzjxm --- crates/validator/src/beacon_node/fallback.rs | 74 ++- crates/validator/src/beacon_node/http.rs | 13 +- crates/validator/src/beacon_node/mock.rs | 16 +- crates/validator/src/beacon_node/mod.rs | 20 +- crates/validator/src/in_flight.rs | 13 +- crates/validator/src/metrics.rs | 6 +- crates/validator/src/preferences.rs | 530 +++++++++++++++---- docs/cli.md | 5 +- docs/metrics.md | 4 +- 9 files changed, 544 insertions(+), 137 deletions(-) diff --git a/crates/validator/src/beacon_node/fallback.rs b/crates/validator/src/beacon_node/fallback.rs index 7f4e6f04f..014b8bc14 100644 --- a/crates/validator/src/beacon_node/fallback.rs +++ b/crates/validator/src/beacon_node/fallback.rs @@ -23,8 +23,8 @@ use crate::beacon_node::dto::{ SyncCommitteeSubscriptionDto, SyncDutyDto, }; use crate::beacon_node::{ - AggregateAttestation, AttesterDuties, BeaconNodeApi, BlockRequest, Genesis, ProposerDuties, - PtcDuties, Published, SignedAggregates, ValidatorEntry, + AggregateAttestation, AttesterDuties, BeaconNodeApi, BlockRequest, Delivery, Genesis, + ProposerDuties, PtcDuties, Published, SignedAggregates, ValidatorEntry, }; use crate::error::{Error, Result}; @@ -65,6 +65,13 @@ impl FallbackBeaconNode { } } + /// The nodes, in order, for a test outside this module that drives a + /// service through the fallback and then inspects each mock. + #[cfg(test)] + pub(crate) fn nodes(&self) -> &[B] { + &self.nodes + } + /// Try each node in order, returning the first success. /// /// "Success" means the node returned `Ok`, nothing more. That is the right @@ -190,6 +197,7 @@ impl FallbackBeaconNode { { self.try_all_inner(what, node_effect, all_failed, true, call) .await + .map(|_| ()) } /// [`Self::try_all`] logging each node's refusal at `debug!` instead of @@ -197,36 +205,43 @@ impl FallbackBeaconNode { /// (once per entry): a per-node warning here would repeat on every retry /// for as long as the refusal lasts. Always without an `all_failed` /// message, for the same reason. - async fn try_all_quiet<'p, F>( + /// + /// Answers whether every node accepted, rather than only whether one did: + /// a caller that retries is one that can send again to the nodes that + /// missed it, and needs to know there are some. + async fn try_all_quiet<'p, T, F>( &'p self, what: &'static str, node_effect: &'static str, call: F, - ) -> Result<()> + ) -> Result where - F: Fn(&'p B) -> BoxFuture<'p, Result<()>>, + F: Fn(&'p B) -> BoxFuture<'p, Result>, { self.try_all_inner(what, node_effect, None, false, call) .await } - async fn try_all_inner<'p, F>( + /// The shared body of [`Self::try_all`] and [`Self::try_all_quiet`]. A + /// node's `Ok` value is not looked into: each node here is one node, so + /// its acceptance is the whole of what it can report. + async fn try_all_inner<'p, T, F>( &'p self, what: &'static str, node_effect: &'static str, all_failed: Option<&'static str>, warn_per_node: bool, call: F, - ) -> Result<()> + ) -> Result where - F: Fn(&'p B) -> BoxFuture<'p, Result<()>>, + F: Fn(&'p B) -> BoxFuture<'p, Result>, { let answers = futures_util::future::join_all(self.nodes.iter().map(&call)).await; let mut accepted = 0; let mut last = None; for (position, answer) in answers.into_iter().enumerate() { match answer { - Ok(()) => accepted += 1, + Ok(_) => accepted += 1, Err(err) => { if warn_per_node { warn!(%what, position, %err, node_effect, "Beacon node did not accept"); @@ -237,8 +252,11 @@ impl FallbackBeaconNode { } } } + if accepted == self.nodes.len() { + return Ok(Delivery::Everywhere); + } if accepted > 0 { - return Ok(()); + return Ok(Delivery::Partly); } let detail = last .map(|err| err.to_string()) @@ -442,12 +460,14 @@ impl BeaconNodeApi for FallbackBeaconNode { /// To every node, since each one gossips what it accepts: a preference /// only the first node heard reaches the mesh through that node alone, and - /// builders subscribed elsewhere would ignore the slot. + /// builders subscribed elsewhere would ignore the slot. [`Delivery::Partly`] + /// when some node refused, so the service sends it again until every node + /// holds it. async fn submit_proposer_preferences( &self, fork: ForkName, preferences: &[gloas::SignedProposerPreferences], - ) -> Result<()> { + ) -> Result { // Quiet: the service retries a refused preference every slot and warns // once per entry, with what it costs. self.try_all_quiet( @@ -1224,10 +1244,11 @@ mod tests { MockBeaconNode::new(), ]); - fallback + let delivery = fallback .submit_proposer_preferences(ForkName::Fulu, std::slice::from_ref(&preference)) .await .expect("two of three accepted"); + assert_eq!(delivery, Delivery::Partly, "the middle node missed it"); for position in [0, 2] { assert_eq!( @@ -1248,6 +1269,33 @@ mod tests { ); } + #[tokio::test] + async fn proposer_preferences_every_node_accepted_are_delivered_everywhere() { + let fallback = FallbackBeaconNode::new(vec![MockBeaconNode::new(), MockBeaconNode::new()]); + let preference = gloas::SignedProposerPreferences::default(); + + let delivery = fallback + .submit_proposer_preferences(ForkName::Fulu, std::slice::from_ref(&preference)) + .await + .expect("both accepted"); + + assert_eq!(delivery, Delivery::Everywhere); + } + + #[tokio::test] + async fn proposer_preferences_no_node_accepted_are_an_error() { + let fallback = FallbackBeaconNode::new(vec![ + MockBeaconNode::failing("down"), + MockBeaconNode::failing("down too"), + ]); + let preference = gloas::SignedProposerPreferences::default(); + + fallback + .submit_proposer_preferences(ForkName::Fulu, std::slice::from_ref(&preference)) + .await + .expect_err("nobody holds it"); + } + #[tokio::test] async fn registrations_reach_every_node() { use ethlambda_types::beacon::builder::{ diff --git a/crates/validator/src/beacon_node/http.rs b/crates/validator/src/beacon_node/http.rs index 9d9789343..724b36562 100644 --- a/crates/validator/src/beacon_node/http.rs +++ b/crates/validator/src/beacon_node/http.rs @@ -46,8 +46,8 @@ use crate::beacon_node::dto::{ VersionedResponse, config_from_spec_response, encode_hex, parse_pubkey, parse_root, }; use crate::beacon_node::{ - AggregateAttestation, AggregateKind, AttesterDuties, BeaconNodeApi, BlockRequest, Genesis, - ProposerDuties, PtcDuties, Published, SignedAggregates, ValidatorEntry, + AggregateAttestation, AggregateKind, AttesterDuties, BeaconNodeApi, BlockRequest, Delivery, + Genesis, ProposerDuties, PtcDuties, Published, SignedAggregates, ValidatorEntry, validate_attestation_data, validate_produced_block, validate_sync_contribution, }; use crate::error::{BeaconNodeFailure, Error, Result}; @@ -885,13 +885,14 @@ impl BeaconNodeApi for HttpBeaconNode { /// JSON, which every beacon node must accept, an empty list never reaching /// the wire. The gloas container serialises as the endpoint's JSON itself. + /// One node, so an acceptance is always [`Delivery::Everywhere`]. async fn submit_proposer_preferences( &self, fork: ForkName, preferences: &[gloas::SignedProposerPreferences], - ) -> Result<()> { + ) -> Result { if preferences.is_empty() { - return Ok(()); + return Ok(Delivery::Everywhere); } self.post_no_content( "/eth/v1/validator/proposer_preferences", @@ -899,6 +900,7 @@ impl BeaconNodeApi for HttpBeaconNode { Some(fork.as_str()), ) .await + .map(|()| Delivery::Everywhere) } async fn execution_payload_envelope( @@ -1625,11 +1627,12 @@ mod tests { signature: BlsSignature([5; 96]), }]; let served = serve(200, vec![], vec![]).await; - served + let delivery = served .node .submit_proposer_preferences(ForkName::Fulu, &preferences) .await .expect("accepted"); + assert_eq!(delivery, Delivery::Everywhere, "one node is every node"); let seen = served.seen.lock().expect("lock"); assert_eq!(seen.len(), 1); diff --git a/crates/validator/src/beacon_node/mock.rs b/crates/validator/src/beacon_node/mock.rs index 66fe3b9fe..929e9bd41 100644 --- a/crates/validator/src/beacon_node/mock.rs +++ b/crates/validator/src/beacon_node/mock.rs @@ -24,8 +24,8 @@ use crate::beacon_node::dto::{ SingleAttestationDto, SyncCommitteeSubscriptionDto, SyncDutyDto, }; use crate::beacon_node::{ - AggregateAttestation, AggregateKind, AttesterDuties, BeaconNodeApi, BlockRequest, Genesis, - ProposerDuties, PtcDuties, Published, SignedAggregates, ValidatorEntry, + AggregateAttestation, AggregateKind, AttesterDuties, BeaconNodeApi, BlockRequest, Delivery, + Genesis, ProposerDuties, PtcDuties, Published, SignedAggregates, ValidatorEntry, validate_sync_contribution, }; use crate::error::{BeaconNodeFailure, Error, Result}; @@ -66,6 +66,11 @@ pub struct MockBeaconNode { pub proposers: Mutex>, /// What `proposer_duties_v2` answers with, by epoch, like `proposers`. pub proposers_v2: Mutex>, + /// Run inside every `proposer_duties_v2` call that gets past the + /// configured failures, before it answers: the point where a caller has + /// taken its own snapshot and waits on the node, so a test can change the + /// world under it there. + pub on_proposer_duties_v2: Option>, /// Every proposer preferences batch `submit_proposer_preferences` /// received, with the fork it was sent under, in call order. pub submitted_preferences: Mutex)>>, @@ -708,6 +713,9 @@ impl BeaconNodeApi for MockBeaconNode { async fn proposer_duties_v2(&self, epoch: Epoch) -> Result { self.guard("proposer_duties_v2")?; *self.proposer_duties_v2_calls.lock().expect("lock") += 1; + if let Some(hook) = &self.on_proposer_duties_v2 { + hook(); + } self.proposers_v2 .lock() .expect("lock") @@ -874,7 +882,7 @@ impl BeaconNodeApi for MockBeaconNode { &self, fork: ForkName, preferences: &[gloas::SignedProposerPreferences], - ) -> Result<()> { + ) -> Result { self.guard("submit_proposer_preferences")?; let refused = self.refused_preference_slots.lock().expect("lock").clone(); if let Some(entry) = preferences @@ -893,7 +901,7 @@ impl BeaconNodeApi for MockBeaconNode { .lock() .expect("lock") .push((fork, preferences.to_vec())); - Ok(()) + Ok(Delivery::Everywhere) } async fn execution_payload_envelope( diff --git a/crates/validator/src/beacon_node/mod.rs b/crates/validator/src/beacon_node/mod.rs index 281ac215d..18fff1595 100644 --- a/crates/validator/src/beacon_node/mod.rs +++ b/crates/validator/src/beacon_node/mod.rs @@ -162,6 +162,21 @@ pub enum Published { BuilderUnconfirmed, } +/// How many of the configured nodes took something sent to every node. +/// +/// The caller needs it when what it sends lives on each node separately and +/// a node that missed it never hears it any other way. Proposer preferences +/// are such a send: each node gossips only what it accepted, and the service +/// stops sending a preference once it counts as delivered, so "some node +/// accepted" must not be mistaken for "every node did". +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Delivery { + /// Every configured node accepted. The only answer a single node gives. + Everywhere, + /// At least one node accepted and at least one refused or did not answer. + Partly, +} + /// Check that `block` is the block that was asked for. /// /// The proposal-shaped counterpart to [`validate_attestation_data`], enforced @@ -437,12 +452,13 @@ pub trait BeaconNodeApi: Send + Sync { /// holds for the same `(slot, dependent_root)` with other content fails /// every call that carries it. The caller therefore sends one preference /// per call. State on the node's gossip, so failover sends it to every - /// node. + /// node, and answers [`Delivery::Partly`] when only some of them accepted + /// it, so the caller knows to send it again. async fn submit_proposer_preferences( &self, fork: ForkName, preferences: &[gloas::SignedProposerPreferences], - ) -> Result<()>; + ) -> Result; /// The envelope the node cached for the self-built payload of the block /// whose root is `block_root`, proposed at `slot`. diff --git a/crates/validator/src/in_flight.rs b/crates/validator/src/in_flight.rs index 62ab3225e..ffbac8b21 100644 --- a/crates/validator/src/in_flight.rs +++ b/crates/validator/src/in_flight.rs @@ -17,7 +17,14 @@ impl InFlight { /// The guard for a new run, or `None` when one is already going. pub(crate) fn enter(&self) -> Option> { let running = self.0.swap(true, Ordering::AcqRel); - (!running).then_some(InFlightGuard(&self.0)) + // The guard is built only for a run that starts: `then_some` would + // build it either way and drop it on the refused path, and dropping + // it clears the flag of the run that is still going. + if running { + None + } else { + Some(InFlightGuard(&self.0)) + } } } @@ -40,6 +47,10 @@ mod tests { let in_flight = InFlight::default(); let first = in_flight.enter().expect("nothing is running"); assert!(in_flight.enter().is_none()); + assert!( + in_flight.enter().is_none(), + "a refused run leaves the running one's flag set" + ); drop(first); assert!(in_flight.enter().is_some()); } diff --git a/crates/validator/src/metrics.rs b/crates/validator/src/metrics.rs index 16c8539bb..9a48af7fb 100644 --- a/crates/validator/src/metrics.rs +++ b/crates/validator/src/metrics.rs @@ -135,7 +135,8 @@ static BUILDER_REGISTRATION_FAILURES_TOTAL: LazyLock = LazyLock::new }); /// Proposer preferences a beacon node accepted. Counted per preference, so a -/// full epoch's worth reads as that many. +/// full epoch's worth reads as that many, and once, when the first node +/// accepts it: the re-sends to nodes that missed it do not count again. static PROPOSER_PREFERENCES_PUBLISHED_TOTAL: LazyLock = LazyLock::new(|| { register_int_counter!( "ethlambda_validator_proposer_preferences_published_total", @@ -146,7 +147,8 @@ static PROPOSER_PREFERENCES_PUBLISHED_TOTAL: LazyLock = LazyLock::ne /// Proposer preferences no beacon node accepted, one count per preference per /// attempt. The next slot retries, so this is a warning sign: until one is -/// accepted, builders' bids for the slot are ignored. +/// accepted, builders' bids for the slot are ignored. A re-send of one some +/// node already accepted is not counted, since that cost does not apply. static PROPOSER_PREFERENCES_FAILURES_TOTAL: LazyLock = LazyLock::new(|| { register_int_counter!( "ethlambda_validator_proposer_preferences_failures_total", diff --git a/crates/validator/src/preferences.rs b/crates/validator/src/preferences.rs index a72b19c28..2a5808a5a 100644 --- a/crates/validator/src/preferences.rs +++ b/crates/validator/src/preferences.rs @@ -32,9 +32,10 @@ //! [`PreferencesService::poll`] runs each slot rather than each epoch. A reorg //! changes a dependent root, which makes the old preference unusable and needs //! a new one; a validator can become active, or be given a fee recipient, -//! mid-epoch; and a node that refused a send must be tried again. What was -//! already accepted is remembered per `(slot, dependent_root)`, so a quiet -//! slot costs the two duty fetches and no signing. +//! mid-epoch; and a node that refused a send must be tried again. What every +//! node accepted is remembered per `(slot, dependent_root)`, and what only some +//! accepted is kept as signed, so a quiet slot costs the two duty fetches and +//! no signing. //! //! # Why every node gets them //! @@ -42,6 +43,17 @@ //! them: a preference only one node heard reaches builders through that node's //! mesh alone. //! +//! So a preference is done only once every node accepted it. A node that was +//! syncing, or lacked the dependent block, when it was first sent never gets +//! it from gossip, which does not replay it; until every node has it, each +//! slot sends it again to all of them. That is safe for the nodes that already +//! hold it: ethlambda's answers 200 for an entry identical to the one it holds, +//! and Lighthouse's treats it as already seen. It is identical because it is +//! the message as first signed, kept rather than signed again: a fee recipient +//! changed in between would otherwise give the late node a second preference +//! for the slot, which the nodes holding the first refuse. The published count +//! takes a preference once, when the first node accepts it. +//! //! # No replacement //! //! A node deduplicates on `(slot, dependent_root)`, so a preference cannot be @@ -64,43 +76,39 @@ //! failed duties fetch, is logged (the fetch once per epoch) and counted, and //! the next slot retries. -use std::collections::HashSet; -use std::sync::atomic::{AtomicBool, Ordering}; +use std::collections::{HashMap, HashSet}; use std::sync::{Arc, Mutex, MutexGuard}; use ethlambda_types::beacon::containers::gloas::{ProposerPreferences, SignedProposerPreferences}; use ethlambda_types::beacon::fork::ForkName; use ethlambda_types::beacon::preset; use ethlambda_types::beacon::primitives::{BlsPubkey, Epoch, Root, Slot}; -use ethlambda_types::beacon::signing::compute_epoch_at_slot; +use ethlambda_types::beacon::signing::{compute_epoch_at_slot, compute_start_slot_at_epoch}; use tokio::sync::RwLock; use tracing::{debug, info, warn}; -use crate::beacon_node::BeaconNodeApi; -use crate::beacon_node::dto::{encode_hex, parse_pubkey}; +use crate::beacon_node::dto::{ProposerDutyDto, encode_hex, parse_pubkey}; +use crate::beacon_node::{BeaconNodeApi, Delivery}; +use crate::in_flight::InFlight; use crate::keys::ValidatorStore; use crate::proposer_settings::ProposerSettings; use crate::signing::SigningContext; -/// Clears [`PreferencesService`]'s `in_flight` flag on every exit from a poll, -/// a panic included, so one failed poll cannot stop every later one. -struct InFlightGuard<'a>(&'a AtomicBool); - -impl Drop for InFlightGuard<'_> { - fn drop(&mut self) { - self.0.store(false, Ordering::Release); - } -} - /// Signs and publishes proposer preferences. pub struct PreferencesService { beacon_node: Arc, context: Arc, settings: Arc, - /// `(proposal_slot, dependent_root)` pairs a node accepted, so a - /// preference is signed and sent once per dependent root. A reorg that - /// moves the root sends the slot again; pruned below the current epoch. + /// `(proposal_slot, dependent_root)` pairs every node accepted, so a + /// preference is signed once per dependent root and sent until all of + /// them hold it. A reorg that moves the root sends the slot again; pruned + /// below the current epoch. published: Mutex>, + /// Preferences some node accepted and another did not, by + /// `(proposal_slot, dependent_root)`, as signed: each poll sends this + /// same message to every node again until all of them accepted it, when + /// it moves to `published`. Pruned with `published`. + partly_published: Mutex>, /// Epochs whose missing fee recipients were already warned about, so the /// warning is once per epoch rather than once per slot. Pruned like /// `published`. @@ -113,7 +121,7 @@ pub struct PreferencesService { /// reason: a syncing node, or one without v2, fails every slot. fetch_warned: Mutex>, /// One poll at a time: a slow node must not stack polls up. - in_flight: AtomicBool, + in_flight: InFlight, } /// Lock `mutex`, recovering a poisoned one: every write here is one set @@ -135,10 +143,11 @@ impl PreferencesService { context, settings, published: Mutex::new(HashSet::new()), + partly_published: Mutex::new(HashMap::new()), no_fee_warned: Mutex::new(HashSet::new()), refusal_warned: Mutex::new(HashSet::new()), fetch_warned: Mutex::new(HashSet::new()), - in_flight: AtomicBool::new(false), + in_flight: InFlight::default(), } } @@ -148,10 +157,9 @@ impl PreferencesService { /// /// Returns at once when the previous poll is still running. pub async fn poll(self: Arc, current_slot: Slot, store: Arc>) { - if self.in_flight.swap(true, Ordering::AcqRel) { + let Some(_running) = self.in_flight.enter() else { return; - } - let _guard = InFlightGuard(&self.in_flight); + }; let epoch = compute_epoch_at_slot(current_slot); // Snapshotted with the guard dropped at once: nothing below awaits a @@ -162,25 +170,39 @@ impl PreferencesService { if self.context.config.fork_at_epoch(target) < ForkName::Gloas { continue; } - self.publish_epoch(current_slot, target, &pubkeys, &store) + let published = self + .publish_epoch(current_slot, target, &pubkeys, &store) .await; + if published > 0 { + crate::metrics::inc_proposer_preferences_published(published); + info!( + epoch = target, + count = published, + "Proposer preferences published" + ); + } } let epoch_start = epoch * preset::SLOTS_PER_EPOCH; lock(&self.published).retain(|(slot, _)| *slot >= epoch_start); + lock(&self.partly_published).retain(|(slot, _), _| *slot >= epoch_start); lock(&self.refusal_warned).retain(|(slot, _)| *slot >= epoch_start); lock(&self.no_fee_warned).retain(|warned| *warned >= epoch); lock(&self.fetch_warned).retain(|warned| *warned >= epoch); } - /// One target epoch: fetch its v2 duties, sign what is still owed and send it. + /// One target epoch: fetch its v2 duties, sign what is still owed, and + /// send it along with what some node still lacks. Returns how many + /// preferences a node accepted for the first time, which is all the + /// published count takes: a re-send to the nodes that missed one is not + /// counted again. async fn publish_epoch( &self, current_slot: Slot, target: Epoch, pubkeys: &HashSet, store: &Arc>, - ) { + ) -> usize { let duties = match self.beacon_node.proposer_duties_v2(target).await { Ok(duties) => duties, Err(err) => { @@ -194,13 +216,15 @@ impl PreferencesService { for this epoch wait for a node that does" ); } - return; + return 0; } }; + let dependent_root = duties.dependent_root; let mut owed = Vec::new(); + let mut resend = Vec::new(); let mut no_fee_recipient = 0usize; - for duty in &duties.duties { + for duty in within_epoch(target, duties.duties) { // Started slots are skipped: gossip would IGNORE them. if duty.slot <= current_slot { continue; @@ -211,21 +235,30 @@ impl PreferencesService { if !pubkeys.contains(&pubkey) { continue; } - if lock(&self.published).contains(&(duty.slot, duties.dependent_root)) { + let key = (duty.slot, dependent_root); + if lock(&self.published).contains(&key) { + continue; + } + if let Some(held) = lock(&self.partly_published).get(&key) { + resend.push(held.clone()); continue; } - let Some(fee_recipient) = self.settings.fee_recipient(&pubkey) else { + // One read for both, so a keymanager write between two reads + // cannot pair an old address with a new gas limit. + let Some((fee_recipient, target_gas_limit)) = + self.settings.fee_recipient_and_gas_limit(&pubkey) + else { no_fee_recipient += 1; continue; }; owed.push(( pubkey, ProposerPreferences { - dependent_root: duties.dependent_root, + dependent_root, proposal_slot: duty.slot, validator_index: duty.validator_index, fee_recipient, - target_gas_limit: self.settings.gas_limit(&pubkey), + target_gas_limit, }, )); } @@ -237,89 +270,162 @@ impl PreferencesService { "Validators without a fee recipient publish no proposer preferences" ); } - if owed.is_empty() { - return; - } - // At most a few per epoch, so signing inline is cheap; the read guard - // is released before the submits. - let mut signed = Vec::with_capacity(owed.len()); - { - let store = store.read().await; - for (pubkey, message) in owed { - match self - .context - .sign_proposer_preferences(&store, &pubkey, &message) - { - Ok(signature) => signed.push(SignedProposerPreferences { message, signature }), - Err(err) => { - warn!( - pubkey = %encode_hex(&pubkey.0), - slot = message.proposal_slot, - %err, - "Could not sign proposer preferences; skipping them" - ); - } - } - } - } + let signed = self.sign(store, owed).await; // The target epoch's fork, which is what the submitted objects belong // to (always gloas here). let fork = self.context.config.fork_at_epoch(target); - let mut published = 0usize; + for entry in resend { + self.submit(fork, target, entry, true).await; + } + let mut first_accepted = 0usize; for entry in signed { - // One request per preference: a node answers 400 for the whole - // call when any entry fails, and the one it refuses (for a - // `(slot, root)` it holds with other content, say) would - // otherwise keep the others from ever being marked published. - let key = (entry.message.proposal_slot, entry.message.dependent_root); + if self.submit(fork, target, entry, false).await { + first_accepted += 1; + } + } + first_accepted + } + + /// Sign each of `owed`, skipping with a warning any it cannot sign, such as + /// one whose key the store no longer holds: the keymanager may have deleted + /// it since the poll took its snapshot of the keys. The others are signed + /// all the same. + /// + /// At most one per slot of the epoch (see [`within_epoch`]), so signing + /// inline is cheap. The read guard is released before the submits, and is + /// not taken at all when nothing is owed, so a quiet slot neither waits on + /// nor holds up a keymanager write. + async fn sign( + &self, + store: &Arc>, + owed: Vec<(BlsPubkey, ProposerPreferences)>, + ) -> Vec { + if owed.is_empty() { + return Vec::new(); + } + let store = store.read().await; + let mut signed = Vec::with_capacity(owed.len()); + for (pubkey, message) in owed { match self - .beacon_node - .submit_proposer_preferences(fork, std::slice::from_ref(&entry)) - .await + .context + .sign_proposer_preferences(&store, &pubkey, &message) { - Ok(()) => { - lock(&self.published).insert(key); - published += 1; - } + Ok(signature) => signed.push(SignedProposerPreferences { message, signature }), Err(err) => { - // A refused entry is tried again every slot until its slot - // passes, so only its first refusal is a warning. - if lock(&self.refusal_warned).insert(key) { - warn!( - epoch = target, - slot = key.0, - validator = entry.message.validator_index, - %err, - "No beacon node accepted a proposer preference; builders' bids for \ - that slot are ignored, and from gloas a node may not know the fee \ - recipient" - ); - } else { - debug!( - epoch = target, - slot = key.0, - validator = entry.message.validator_index, - %err, - "A proposer preference was refused again" - ); - } - crate::metrics::inc_proposer_preferences_failures(); + warn!( + pubkey = %encode_hex(&pubkey.0), + slot = message.proposal_slot, + %err, + "Could not sign proposer preferences; skipping them" + ); } } } - if published > 0 { - crate::metrics::inc_proposer_preferences_published(published); - info!( - epoch = target, - count = published, - "Proposer preferences published" - ); + signed + } + + /// Send one preference to every node and record what came of it. + /// `resent` marks one some node already accepted, taken from + /// `partly_published`. Returns whether a node accepted it for the first + /// time. + async fn submit( + &self, + fork: ForkName, + target: Epoch, + entry: SignedProposerPreferences, + resent: bool, + ) -> bool { + // One request per preference: a node answers 400 for the whole call + // when any entry fails, and the one it refuses (for a `(slot, root)` it + // holds with other content, say) would otherwise keep the others from + // ever being marked published. + let key = (entry.message.proposal_slot, entry.message.dependent_root); + let answer = self + .beacon_node + .submit_proposer_preferences(fork, std::slice::from_ref(&entry)) + .await; + match answer { + Ok(Delivery::Everywhere) => { + lock(&self.partly_published).remove(&key); + lock(&self.published).insert(key); + !resent + } + Ok(Delivery::Partly) => { + // Kept as signed, so the nodes that missed it get this very + // message next slot. The fallback logged each refusal at debug. + lock(&self.partly_published).insert(key, entry); + !resent + } + Err(err) if resent => { + // Not a failure in the warning's or the metric's sense: a node + // already gossiped it. It stays kept for the next slot. + debug!( + epoch = target, + slot = key.0, + validator = entry.message.validator_index, + %err, + "No beacon node accepted a proposer preference sent again" + ); + false + } + Err(err) => { + // A refused entry is tried again every slot until its slot + // passes, so only its first refusal is a warning. + if lock(&self.refusal_warned).insert(key) { + warn!( + epoch = target, + slot = key.0, + validator = entry.message.validator_index, + %err, + "No beacon node accepted a proposer preference; builders' bids for \ + that slot are ignored, and from gloas a node may not know the fee \ + recipient" + ); + } else { + debug!( + epoch = target, + slot = key.0, + validator = entry.message.validator_index, + %err, + "A proposer preference was refused again" + ); + } + crate::metrics::inc_proposer_preferences_failures(); + false + } } } } +/// `duties` cut to what one epoch can hold: those whose slot is in `target`, +/// and of those only the first listed for each slot. +/// +/// The duties are a node's answer, so a buggy or hostile node chooses how +/// many there are and which slots they name. Every one left that names this +/// client's key is signed under the store's read guard, sent in a request of +/// its own, and remembered under a `(slot, dependent_root)` key the prune +/// keeps for as long as its slot lies ahead. Cut here, a target epoch costs at +/// most `SLOTS_PER_EPOCH` of each, whatever the node answered. +fn within_epoch(target: Epoch, duties: Vec) -> Vec { + let slots = compute_start_slot_at_epoch(target)..compute_start_slot_at_epoch(target + 1); + let listed = duties.len(); + let mut seen = HashSet::new(); + let kept: Vec = duties + .into_iter() + .filter(|duty| slots.contains(&duty.slot) && seen.insert(duty.slot)) + .collect(); + let dropped = listed - kept.len(); + if dropped > 0 { + debug!( + epoch = target, + dropped, "Dropped out-of-epoch or repeated-slot proposer duties" + ); + } + kept +} + #[cfg(test)] mod tests { use super::*; @@ -708,7 +814,7 @@ mod tests { ); let (service, node) = service(node, settings(Some(FEE_RECIPIENT))); - service.in_flight.store(true, Ordering::Release); + let running = service.in_flight.enter().expect("nothing is running"); service .clone() .poll(slot_in(GLOAS_EPOCH, 1), store.clone()) @@ -716,16 +822,226 @@ mod tests { assert!(submitted(&node).is_empty()); assert_eq!(node.proposer_duties_v2_call_count(), 0); assert!( - service.in_flight.load(Ordering::Acquire), + service.in_flight.enter().is_none(), "the running poll's flag is not cleared by the one that bounced" ); - service.in_flight.store(false, Ordering::Release); + drop(running); service.clone().poll(slot_in(GLOAS_EPOCH, 1), store).await; assert_eq!(submitted(&node).len(), 1); + assert!(service.in_flight.enter().is_some(), "cleared after a poll"); + } + + /// A duty is signed for only under the epoch it belongs to: one a node + /// lists under another epoch, or far in the future, is dropped, and leaves + /// nothing behind for the prune to miss. + #[tokio::test] + async fn duties_outside_the_target_epoch_are_dropped() { + let (store, pubkeys) = store_with_keys(1); + let store = Arc::new(RwLock::new(store)); + let this_epoch = slot_in(GLOAS_EPOCH, 10); + let next_epoch = slot_in(GLOAS_EPOCH + 1, 4); + let node = MockBeaconNode::new() + .with_proposers_v2( + GLOAS_EPOCH, + Root::repeat_byte(0x11), + vec![ + duty(&pubkeys[0], 3, this_epoch), + duty(&pubkeys[0], 3, slot_in(GLOAS_EPOCH + 1, 5)), + duty(&pubkeys[0], 3, slot_in(GLOAS_EPOCH + 1_000, 0)), + ], + ) + .with_proposers_v2( + GLOAS_EPOCH + 1, + Root::repeat_byte(0x22), + vec![ + // Unstarted, but the epoch before the one asked for. + duty(&pubkeys[0], 3, slot_in(GLOAS_EPOCH, 20)), + duty(&pubkeys[0], 3, next_epoch), + ], + ); + let (service, node) = service(node, settings(Some(FEE_RECIPIENT))); + + service.clone().poll(slot_in(GLOAS_EPOCH, 1), store).await; + + let mut sent: Vec<(Slot, Root)> = submitted(&node) + .iter() + .flat_map(|(_, entries)| entries.iter()) + .map(|entry| (entry.message.proposal_slot, entry.message.dependent_root)) + .collect(); + sent.sort(); + let expected = vec![ + (this_epoch, Root::repeat_byte(0x11)), + (next_epoch, Root::repeat_byte(0x22)), + ]; + assert_eq!(sent, expected); + assert_eq!( + *lock(&service.published), + expected.into_iter().collect::>() + ); + } + + /// However many duties a node lists, an epoch signs at most one preference + /// per slot, for the duty listed first. + #[tokio::test] + async fn hundreds_of_duties_sign_at_most_one_preference_per_slot() { + let (store, pubkeys) = store_with_keys(1); + let store = Arc::new(RwLock::new(store)); + // Every even entry names a slot of the gloas epoch, cycling through + // it; every odd one a slot epochs ahead. + let duties = (0..400) + .map(|position| { + let slot = if position % 2 == 0 { + slot_in(GLOAS_EPOCH, (position / 2) % preset::SLOTS_PER_EPOCH) + } else { + slot_in(GLOAS_EPOCH + 1 + position, 0) + }; + duty(&pubkeys[0], position, slot) + }) + .collect(); + let node = + MockBeaconNode::new().with_proposers_v2(GLOAS_EPOCH, Root::repeat_byte(0x11), duties); + let (service, node) = service(node, settings(Some(FEE_RECIPIENT))); + + // The last slot before gloas: the current epoch is skipped, and every + // slot of the gloas epoch is still ahead. + let current = slot_in(GLOAS_EPOCH - 1, preset::SLOTS_PER_EPOCH - 1); + service.poll(current, store).await; + + let mut sent: Vec<(Slot, u64)> = submitted(&node) + .iter() + .inspect(|(_, entries)| assert_eq!(entries.len(), 1, "one per request")) + .map(|(_, entries)| { + let message = &entries[0].message; + (message.proposal_slot, message.validator_index) + }) + .collect(); + sent.sort(); + let first_listed: Vec<(Slot, u64)> = (0..preset::SLOTS_PER_EPOCH) + .map(|offset| (slot_in(GLOAS_EPOCH, offset), 2 * offset)) + .collect(); + assert_eq!(sent, first_listed); + } + + /// A node that refused a preference a peer accepted gets the same signed + /// message on a later poll, once it accepts; the preference is counted as + /// published once, at the first acceptance, and is not sent again after + /// every node holds it. + #[tokio::test] + async fn a_node_that_refused_gets_the_preference_once_it_accepts() { + use crate::beacon_node::fallback::FallbackBeaconNode; + + let (store, pubkeys) = store_with_keys(1); + let snapshot: HashSet = pubkeys.iter().copied().collect(); + let store = Arc::new(RwLock::new(store)); + let slot = slot_in(GLOAS_EPOCH, 10); + let root = Root::repeat_byte(0x11); + let node = || { + MockBeaconNode::new().with_proposers_v2( + GLOAS_EPOCH, + root, + vec![duty(&pubkeys[0], 3, slot)], + ) + }; + let refusing = node(); + refusing + .refused_preference_slots + .lock() + .expect("lock") + .push(slot); + let fallback = Arc::new(FallbackBeaconNode::new(vec![node(), refusing])); + let service = + PreferencesService::new(fallback.clone(), context(), settings(Some(FEE_RECIPIENT))); + let held = |position: usize| { + fallback.nodes()[position] + .submitted_preferences + .lock() + .expect("lock") + .clone() + }; + let current = slot_in(GLOAS_EPOCH, 1); + + let counted = service + .publish_epoch(current, GLOAS_EPOCH, &snapshot, &store) + .await; + assert_eq!(counted, 1, "published once a node took it"); + assert_eq!(held(0).len(), 1); + assert!(held(1).is_empty()); + assert!(lock(&service.published).is_empty(), "not every node has it"); + + // Still refused: sent to both again, not counted again, and no warning + // record, since a node holds it. + let counted = service + .publish_epoch(current + 1, GLOAS_EPOCH, &snapshot, &store) + .await; + assert_eq!(counted, 0); + assert_eq!(held(0).len(), 2); + assert!(held(1).is_empty()); + assert!(lock(&service.refusal_warned).is_empty()); + + fallback.nodes()[1] + .refused_preference_slots + .lock() + .expect("lock") + .clear(); + let counted = service + .publish_epoch(current + 2, GLOAS_EPOCH, &snapshot, &store) + .await; + assert_eq!(counted, 0, "the accepting node's copy is not counted twice"); + let first = held(0)[0].clone(); + assert_eq!(held(1), vec![first], "the message as first signed"); + assert_eq!(*lock(&service.published), HashSet::from([(slot, root)])); + assert!(lock(&service.partly_published).is_empty()); + + // Every node has it: nothing more is sent. + service + .publish_epoch(current + 3, GLOAS_EPOCH, &snapshot, &store) + .await; + assert_eq!(held(0).len(), 3); + assert_eq!(held(1).len(), 1); + } + + /// A key the keymanager deletes while the poll waits on the duties (after + /// the poll's snapshot of the keys, before its signing) is skipped, and + /// the other validators' preferences still go out. + #[tokio::test] + async fn a_key_removed_before_signing_is_skipped_without_losing_the_others() { + let (store, pubkeys) = store_with_keys(2); + let store = Arc::new(RwLock::new(store)); + let kept = slot_in(GLOAS_EPOCH, 11); + let mut node = MockBeaconNode::new().with_proposers_v2( + GLOAS_EPOCH, + Root::repeat_byte(0x11), + vec![ + duty(&pubkeys[0], 3, slot_in(GLOAS_EPOCH, 10)), + duty(&pubkeys[1], 4, kept), + ], + ); + let deleted_from = store.clone(); + let deleted = pubkeys[0]; + node.on_proposer_duties_v2 = Some(Box::new(move || { + deleted_from + .try_write() + .expect("the poll holds no guard while it waits on the node") + .remove(&deleted); + })); + let (service, node) = service(node, settings(Some(FEE_RECIPIENT))); + + service + .clone() + .poll(slot_in(GLOAS_EPOCH, 1), store.clone()) + .await; + assert!( - !service.in_flight.load(Ordering::Acquire), - "cleared after a poll" + !store.read().await.contains(&pubkeys[0]), + "deleted mid-poll" + ); + let sent = submitted(&node); + assert_eq!(sent.len(), 1); + assert_eq!(sent[0].1[0].message.validator_index, 4); + assert_eq!( + *lock(&service.published), + HashSet::from([(kept, Root::repeat_byte(0x11))]) ); } } diff --git a/docs/cli.md b/docs/cli.md index 76c9e25b9..b645b4424 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -325,7 +325,10 @@ slot, whether or not `--builder-proposals` is set: from gloas the preferences are what carry a proposal's fee recipient, and a node ignores builders' bids for a slot with none. It reads those duties from the v2 proposer duties endpoint, whose dependent root the preferences must carry, and publishes each -`(slot, dependent root)` once, again after a reorg changes the root. A +`(slot, dependent root)` until every node has accepted it, sending the message +as first signed to all of them again each slot until then, and again after a +reorg changes the root. Duties outside their epoch, and all but the first for +a slot, are dropped before anything is signed. A validator with no fee recipient is skipped, with one warning per epoch naming the count. A preference cannot be replaced for the same slot and root, so a fee recipient changed through the keymanager API applies from the next diff --git a/docs/metrics.md b/docs/metrics.md index 267b3066c..f21d79c16 100644 --- a/docs/metrics.md +++ b/docs/metrics.md @@ -750,8 +750,8 @@ and absent is not the same as zero. | `ethlambda_validator_fee_recipient_mismatches_total` | Counter | Blocks paying execution rewards to an address this client did not request. Should read zero forever; a non-zero value means every proposal is paying somewhere else | | `ethlambda_validator_builder_registrations_sent_total` | Counter | Builder registrations a beacon node accepted (`--builder-proposals`) | | `ethlambda_validator_builder_registration_failures_total` | Counter | Registration batches no beacon node accepted. A missed registration costs at most a builder block, never a duty | -| `ethlambda_validator_proposer_preferences_published_total` | Counter | Signed proposer preferences a beacon node accepted, counted per preference. Published for every proposal from the epoch before gloas, whether or not `--builder-proposals` is set | -| `ethlambda_validator_proposer_preferences_failures_total` | Counter | Proposer preferences a beacon node refused or failed to take (one count per preference per attempt). The next slot retries; until a preference is accepted, builders' bids for its slot are ignored | +| `ethlambda_validator_proposer_preferences_published_total` | Counter | Signed proposer preferences a beacon node accepted, counted once per preference, when the first node accepts it (re-sends to nodes that missed it do not count). Published for every proposal from the epoch before gloas, whether or not `--builder-proposals` is set | +| `ethlambda_validator_proposer_preferences_failures_total` | Counter | Proposer preferences a beacon node refused or failed to take (one count per preference per attempt). The next slot retries; until a preference is accepted, builders' bids for its slot are ignored. A re-send of one some node already accepted is not counted | | `ethlambda_validator_proposer_preferences_duty_fetch_failures_total` | Counter | v2 proposer-duty fetches no beacon node answered (one count per target epoch per attempt). Without the duties no preference is signed for that epoch; a syncing node, or one without the v2 endpoint, shows up here | | `ethlambda_validator_blinded_blocks_proposed_total` | Counter | Blinded blocks a beacon node handed to its builder. A subset of `blocks_proposed_total` | | `ethlambda_validator_blinded_blocks_unconfirmed_total` | Counter | Blinded blocks the beacon node did not confirm (a 5xx or no answer), which the relay may still have published. Not retried, since a second block for the slot would be an equivocation | From 3d3220ba8110be389fe3fc114c14b06dd50228f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Thu, 5 Nov 2026 18:42:31 -0300 Subject: [PATCH 13/16] docs(validator): say --builder-proposals only sets the gloas boost factor Claude-Session: https://claude.ai/code/session_011vghNneoZ1Ta9W6JNqzjxm --- bin/ethlambda/src/validator.rs | 7 ++++--- docs/cli.md | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/bin/ethlambda/src/validator.rs b/bin/ethlambda/src/validator.rs index 4febabe66..eef2a086a 100644 --- a/bin/ethlambda/src/validator.rs +++ b/bin/ethlambda/src/validator.rs @@ -82,9 +82,10 @@ pub(crate) struct ValidatorOptions { /// Propose builder blocks: register each validator with the beacon node's /// builder and accept a blinded block when the builder's bid wins. Requires /// a beacon node started with a builder (ethlambda: `--builder`). The - /// registrations and blinded blocks are fulu and earlier only; from gloas - /// the builder market works through proposer preferences, and this flag's - /// boost factor weighs p2p bids in `produceBlockV4`. + /// registrations and blinded blocks are fulu and earlier only. From gloas + /// the builder market works through proposer preferences, which are + /// published with or without this flag; there it only sets the boost + /// factor that weighs p2p bids in `produceBlockV4`. #[arg(long, default_value = "false")] pub(crate) builder_proposals: bool, diff --git a/docs/cli.md b/docs/cli.md index 16aeeb85d..b5c1ba211 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -291,7 +291,7 @@ implementation's, so none of the [common flags](#common-flags) apply to it. | `--suggested-fee-recipient` | none | Execution address to receive block rewards, `0x`-prefixed, for every validator the keymanager API gives no address of its own. Optional, and startup warns when it is absent: without it the beacon node picks an address, and it will not be yours. The zero address is accepted, as every other client accepts it, but reported at error at startup: it burns the rewards, and builder registrations and proposer preferences sign it for the builder to pay | | `--graffiti` | empty | Text for the graffiti field of proposed blocks, for every validator the keymanager API gives no graffiti of its own. At most 32 bytes as UTF-8, right-padded with zeros. Refused rather than truncated if longer. An ethlambda beacon node appends both clients' versions to it (see [`rpc.md`](rpc.md#validator-endpoints)) | | `--enable-keymanager` | off | Serve the keymanager API: keystores, plus each validator's own fee recipient, graffiti and gas limit. Off by default because it mutates key material | -| `--builder-proposals` | off | Propose builder blocks: register each validator with the beacon node's builder (an ethlambda node: `--builder`) and accept a blinded block when the builder's bid wins. Registrations and blinded blocks are fulu and earlier only; from gloas the builder market works through proposer preferences, and the boost factor weighs p2p bids in `produceBlockV4` | +| `--builder-proposals` | off | Propose builder blocks: register each validator with the beacon node's builder (an ethlambda node: `--builder`) and accept a blinded block when the builder's bid wins. Registrations and blinded blocks are fulu and earlier only. From gloas the builder market works through proposer preferences, which are published with or without this flag; there it only sets the boost factor that weighs p2p bids in `produceBlockV4` | | `--builder-boost-factor` | `100` | How a builder's bid is weighed against the local payload: the local payload wins iff `local >= factor * bid / 100`. `0` prefers the local payload unless it cannot be built. From gloas it is also sent in `produceBlockV4`'s body, so a pooled p2p bid can win on value; without `--builder-proposals` the body carries `0`. Requires `--builder-proposals`; conflicts with `--prefer-builder-proposals` | | `--prefer-builder-proposals` | off | Take the builder's block whenever there is a valid bid, unless the execution client asks to override the builder (sends `builder_boost_factor = u64::MAX`). Requires `--builder-proposals`; conflicts with `--builder-boost-factor` | From 9fed8e405a3b3c982fcbac70cd487f9f12ede717 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Thu, 12 Nov 2026 18:12:54 -0300 Subject: [PATCH 14/16] fix(validator): re-send proposer preferences once an epoch, all at once One map from (slot, dependent_root) to the signed message and its stage replaces published/partly_published/refusal_warned. It keeps the message after every node accepted it, which the rest needs: - A beacon node holds preferences in memory only, so a restarted one had none for up to two epochs. Every kept preference whose slot is ahead is now sent again, as signed, once an epoch, at the poll of the epoch's last slot, so the next epoch's first slot is covered: a node restarted mid-epoch lacks only the rest of that epoch's. A re-send some node misses is sent each slot from then on; it is never counted as published again. - Sends were one awaited request after another, so one node that does not answer delayed the last of 25 by minutes. Each preference is still its own request, but all of a poll's go out at once, and both target epochs are fetched and sent concurrently: a hung node costs one timeout per poll. With the slot clock, a slot that started while the poll waited is not sent, and one that started in flight is not reported as refused. - Partial delivery is counted: the failures counter takes every send some node refused or did not answer, re-sends included, and the preferences newly held by only some nodes are warned about once per poll. - The pre-sign fee recipient check accepts the address of a preference a node accepted for the slot, which a gloas node pays in preference to the current one, and its hint names the dependent-root delay. Also: the poll gates on gloas and on an empty key set before reading the store or fetching duties, sign's warning puts slot first, and the HTTP client and the mock share one proposer duties lookup for v1 and v2. --- crates/validator/src/beacon_node/http.rs | 28 +- crates/validator/src/beacon_node/mock.rs | 56 +- crates/validator/src/beacon_node/mod.rs | 4 +- crates/validator/src/lib.rs | 19 +- crates/validator/src/metrics.rs | 11 +- crates/validator/src/preferences.rs | 764 ++++++++++++++++++---- crates/validator/src/proposal.rs | 104 ++- crates/validator/src/proposer_settings.rs | 6 + docs/cli.md | 10 +- docs/metrics.md | 2 +- docs/spec_deviations.md | 6 + 11 files changed, 798 insertions(+), 212 deletions(-) diff --git a/crates/validator/src/beacon_node/http.rs b/crates/validator/src/beacon_node/http.rs index d20e8d632..b5147acf1 100644 --- a/crates/validator/src/beacon_node/http.rs +++ b/crates/validator/src/beacon_node/http.rs @@ -560,6 +560,18 @@ impl HttpBeaconNode { }) } + /// The proposer duties for `epoch` at the endpoint's `version` (`v1` or + /// `v2`), which differ only in which `dependent_root` they report. + async fn proposer_duties_at(&self, version: &str, epoch: Epoch) -> Result { + let response: DutiesResponse> = self + .get(&format!("/eth/{version}/validator/duties/proposer/{epoch}")) + .await?; + Ok(ProposerDuties { + dependent_root: parse_root(&response.dependent_root)?, + duties: response.data, + }) + } + async fn decode(response: reqwest::Response) -> Result { let status = response.status(); if status == StatusCode::SERVICE_UNAVAILABLE { @@ -681,25 +693,13 @@ impl BeaconNodeApi for HttpBeaconNode { /// answers for every proposer in the epoch rather than for a submitted /// list, so there is nothing to send. async fn proposer_duties(&self, epoch: Epoch) -> Result { - let response: DutiesResponse> = self - .get(&format!("/eth/v1/validator/duties/proposer/{epoch}")) - .await?; - Ok(ProposerDuties { - dependent_root: parse_root(&response.dependent_root)?, - duties: response.data, - }) + self.proposer_duties_at("v1", epoch).await } /// [`Self::proposer_duties`] against the v2 path, whose `dependent_root` /// is the shuffling dependent root proposer preferences must carry. async fn proposer_duties_v2(&self, epoch: Epoch) -> Result { - let response: DutiesResponse> = self - .get(&format!("/eth/v2/validator/duties/proposer/{epoch}")) - .await?; - Ok(ProposerDuties { - dependent_root: parse_root(&response.dependent_root)?, - duties: response.data, - }) + self.proposer_duties_at("v2", epoch).await } async fn attestation_data(&self, slot: Slot, fork: ForkName) -> Result { diff --git a/crates/validator/src/beacon_node/mock.rs b/crates/validator/src/beacon_node/mock.rs index 929e9bd41..e033fdab7 100644 --- a/crates/validator/src/beacon_node/mock.rs +++ b/crates/validator/src/beacon_node/mock.rs @@ -78,6 +78,9 @@ pub struct MockBeaconNode { /// holds with other content: a call carrying any of them is an `Err` and /// records nothing, like the node's 400 for the whole call. pub refused_preference_slots: Mutex>, + /// How long `submit_proposer_preferences` takes before it answers, as a + /// slow node would. + pub preference_delay: Option, pub attestation_data: Option, /// The slot and proposer the mock will produce a block for. `None` makes /// `produce_block` fail, the way an unset `attestation_data` does. @@ -370,6 +373,32 @@ impl MockBeaconNode { self } + /// Make `submit_proposer_preferences` take `delay` before it answers. + pub fn with_preference_delay(mut self, delay: std::time::Duration) -> Self { + self.preference_delay = Some(delay); + self + } + + /// The duties `stored` holds for `epoch`, or the error a node without them + /// answers, naming `what` was asked for. + fn proposers_at( + stored: &Mutex>, + epoch: Epoch, + what: &str, + ) -> Result { + stored + .lock() + .expect("lock") + .iter() + .find(|(held, _)| *held == epoch) + .map(|(_, duties)| duties.clone()) + .ok_or_else(|| Error::BeaconNode { + url: "mock".to_string(), + failure: BeaconNodeFailure::Request, + detail: format!("no {what} for epoch {epoch}"), + }) + } + /// Make `publish_blinded_block` take `delay` before it records the block. pub fn with_blinded_publish_delay(mut self, delay: std::time::Duration) -> Self { self.blinded_publish_delay = Some(delay); @@ -697,17 +726,7 @@ impl BeaconNodeApi for MockBeaconNode { async fn proposer_duties(&self, epoch: Epoch) -> Result { self.guard("proposer_duties")?; *self.proposer_duties_calls.lock().expect("lock") += 1; - self.proposers - .lock() - .expect("lock") - .iter() - .find(|(stored, _)| *stored == epoch) - .map(|(_, duties)| duties.clone()) - .ok_or_else(|| Error::BeaconNode { - url: "mock".to_string(), - failure: BeaconNodeFailure::Request, - detail: format!("no proposer duties for epoch {epoch}"), - }) + Self::proposers_at(&self.proposers, epoch, "proposer duties") } async fn proposer_duties_v2(&self, epoch: Epoch) -> Result { @@ -716,17 +735,7 @@ impl BeaconNodeApi for MockBeaconNode { if let Some(hook) = &self.on_proposer_duties_v2 { hook(); } - self.proposers_v2 - .lock() - .expect("lock") - .iter() - .find(|(stored, _)| *stored == epoch) - .map(|(_, duties)| duties.clone()) - .ok_or_else(|| Error::BeaconNode { - url: "mock".to_string(), - failure: BeaconNodeFailure::Request, - detail: format!("no v2 proposer duties for epoch {epoch}"), - }) + Self::proposers_at(&self.proposers_v2, epoch, "v2 proposer duties") } async fn attestation_data(&self, slot: Slot, fork: ForkName) -> Result { @@ -884,6 +893,9 @@ impl BeaconNodeApi for MockBeaconNode { preferences: &[gloas::SignedProposerPreferences], ) -> Result { self.guard("submit_proposer_preferences")?; + if let Some(delay) = self.preference_delay { + tokio::time::sleep(delay).await; + } let refused = self.refused_preference_slots.lock().expect("lock").clone(); if let Some(entry) = preferences .iter() diff --git a/crates/validator/src/beacon_node/mod.rs b/crates/validator/src/beacon_node/mod.rs index d2105c4ad..9e864fbe0 100644 --- a/crates/validator/src/beacon_node/mod.rs +++ b/crates/validator/src/beacon_node/mod.rs @@ -179,8 +179,8 @@ pub enum Published { /// The caller needs it when what it sends lives on each node separately and /// a node that missed it never hears it any other way. Proposer preferences /// are such a send: each node gossips only what it accepted, and the service -/// stops sending a preference once it counts as delivered, so "some node -/// accepted" must not be mistaken for "every node did". +/// sends a preference every node holds only once an epoch more, so "some +/// node accepted" must not be mistaken for "every node did". #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Delivery { /// Every configured node accepted. The only answer a single node gives. diff --git a/crates/validator/src/lib.rs b/crates/validator/src/lib.rs index bacd63f07..dbdfe8d61 100644 --- a/crates/validator/src/lib.rs +++ b/crates/validator/src/lib.rs @@ -241,6 +241,14 @@ pub async fn run(config: ValidatorConfig) -> Result<()> { let aggregation = AggregationService::new(beacon_node.clone(), context.clone()); let payload_attestation = PayloadAttestationService::new(beacon_node.clone(), context.clone()); let sync_committee = SyncCommitteeService::new(beacon_node.clone(), context.clone()); + + // Unconditional, as Lighthouse does: from gloas the preferences carry the + // fee recipient of every proposal, builders or not. See the module doc. + let preferences = Arc::new( + PreferencesService::new(beacon_node.clone(), context.clone(), settings.clone()) + .with_clock(clock.clone()), + ); + let proposal = ProposalService::new( beacon_node.clone(), context.clone(), @@ -248,7 +256,8 @@ pub async fn run(config: ValidatorConfig) -> Result<()> { protection.clone(), config.builder.map(|builder| builder.boost_factor), ) - .with_clock(clock.clone()); + .with_clock(clock.clone()) + .with_preferences(preferences.clone()); let registration = config.builder.map(|builder| { info!( boost_factor = builder.boost_factor, @@ -261,14 +270,6 @@ pub async fn run(config: ValidatorConfig) -> Result<()> { )) }); - // Unconditional, as Lighthouse does: from gloas the preferences carry the - // fee recipient of every proposal, builders or not. See the module doc. - let preferences = Arc::new(PreferencesService::new( - beacon_node.clone(), - context.clone(), - settings.clone(), - )); - if let Some(address) = config.keymanager { let token = http_api::load_or_create_token(&config.validators_dir)?; let context = http_api::KeymanagerContext { diff --git a/crates/validator/src/metrics.rs b/crates/validator/src/metrics.rs index b6aea1128..1d0757b59 100644 --- a/crates/validator/src/metrics.rs +++ b/crates/validator/src/metrics.rs @@ -145,14 +145,15 @@ static PROPOSER_PREFERENCES_PUBLISHED_TOTAL: LazyLock = LazyLock::ne .unwrap() }); -/// Proposer preferences no beacon node accepted, one count per preference per -/// attempt. The next slot retries, so this is a warning sign: until one is -/// accepted, builders' bids for the slot are ignored. A re-send of one some -/// node already accepted is not counted, since that cost does not apply. +/// Proposer preference sends some beacon node refused or did not answer, one +/// count per preference per attempt, re-sends included. The next slot retries, +/// so this is a warning sign: a node that misses one never gossips it, and +/// until some node accepts it, builders' bids for the slot are ignored. A send +/// whose slot started before the nodes answered is not counted. static PROPOSER_PREFERENCES_FAILURES_TOTAL: LazyLock = LazyLock::new(|| { register_int_counter!( "ethlambda_validator_proposer_preferences_failures_total", - "Proposer preferences no beacon node accepted" + "Proposer preference sends some beacon node refused or did not answer" ) .unwrap() }); diff --git a/crates/validator/src/preferences.rs b/crates/validator/src/preferences.rs index 2a5808a5a..487b1debe 100644 --- a/crates/validator/src/preferences.rs +++ b/crates/validator/src/preferences.rs @@ -32,10 +32,9 @@ //! [`PreferencesService::poll`] runs each slot rather than each epoch. A reorg //! changes a dependent root, which makes the old preference unusable and needs //! a new one; a validator can become active, or be given a fee recipient, -//! mid-epoch; and a node that refused a send must be tried again. What every -//! node accepted is remembered per `(slot, dependent_root)`, and what only some -//! accepted is kept as signed, so a quiet slot costs the two duty fetches and -//! no signing. +//! mid-epoch; and a node that refused a send must be tried again. What a node +//! accepted is kept per `(slot, dependent_root)`, as signed, so a quiet slot +//! costs the two duty fetches and no signing. //! //! # Why every node gets them //! @@ -54,36 +53,59 @@ //! for the slot, which the nodes holding the first refuse. The published count //! takes a preference once, when the first node accepts it. //! +//! # Again once an epoch +//! +//! A node keeps the preferences it accepted in memory only, so one that +//! restarts has lost them, and gossip does not give them back. Every kept +//! preference whose slot is still ahead is therefore sent again, as signed, +//! once an epoch, as the duty loop re-sends `prepare_beacon_proposer`, which a +//! restart loses too. The round runs at the poll of an epoch's last slot, +//! after which every slot of the next epoch is still ahead, so a node +//! restarted mid-epoch lacks the rest of that epoch's preferences and holds +//! every later one, the next epoch's first slot included. Each node answers +//! the re-send as it does any other: one still holding the preference with a +//! 200 and no republish, one that lost it by taking it again, and one not +//! ready yet by making it a preference only some nodes hold, sent each slot +//! from then on. +//! //! # No replacement //! //! A node deduplicates on `(slot, dependent_root)`, so a preference cannot be //! replaced for the same slot and root. A fee recipient changed after a //! preference went out applies from the next dependent root onward only. //! -//! # One preference per request +//! # One preference per request, all at once //! -//! A node answers 400 for a whole call when any entry in it fails, and one it -//! holds for the same `(slot, dependent_root)` with other content is such a -//! failure (a restart after a fee recipient change, say). Batching would then -//! fail every entry every slot until that slot passed, so each preference is -//! sent alone, and each success is remembered on its own. There are only a -//! handful per epoch. +//! A call has one status for all its entries: a node answers 400 when any +//! entry fails, and one it holds for the same `(slot, dependent_root)` with +//! other content is such a failure (a restart after a fee recipient change, +//! say). Which entries passed is then only as good as the 400's per-entry +//! list. Sent alone, each preference's own status says whether it was +//! accepted, with no body to trust, and each success is remembered on its own. +//! +//! The requests go out together, both target epochs' fetches and sends too, so +//! a node that does not answer costs a poll one duties fetch and one request +//! timeout rather than one timeout per preference. A poll sends at most one +//! preference per slot of the two epochs. //! //! # Failures are never propagated //! //! A missed preference costs a builder bid and, from gloas, possibly the //! node's knowledge of the fee recipient, never a duty. A failed send, or a -//! failed duties fetch, is logged (the fetch once per epoch) and counted, and -//! the next slot retries. +//! failed duties fetch, is logged and counted, and the next slot retries. The +//! fetch warns once per epoch, a preference no node took once, and the +//! preferences some node did not take once per poll that finds new ones. A +//! preference whose slot started before the nodes answered is neither: gossip +//! ignores a started slot, and the poll was late, not refused. use std::collections::{HashMap, HashSet}; use std::sync::{Arc, Mutex, MutexGuard}; use ethlambda_types::beacon::containers::gloas::{ProposerPreferences, SignedProposerPreferences}; use ethlambda_types::beacon::fork::ForkName; -use ethlambda_types::beacon::preset; -use ethlambda_types::beacon::primitives::{BlsPubkey, Epoch, Root, Slot}; +use ethlambda_types::beacon::primitives::{BlsPubkey, Epoch, ExecutionAddress, Root, Slot}; use ethlambda_types::beacon::signing::{compute_epoch_at_slot, compute_start_slot_at_epoch}; +use futures_util::future::join_all; use tokio::sync::RwLock; use tracing::{debug, info, warn}; @@ -93,45 +115,118 @@ use crate::in_flight::InFlight; use crate::keys::ValidatorStore; use crate::proposer_settings::ProposerSettings; use crate::signing::SigningContext; +use crate::slot_clock::SlotClock; + +/// Where one `(proposal_slot, dependent_root)`'s preference stands. +#[derive(Debug, Clone)] +enum Sent { + /// No node has accepted one, and that was warned about. Signed afresh + /// each poll, so an address set in between is the one that goes out. + Refused, + /// A node accepted this message and another may not hold it: one refused + /// or did not answer, or none took the epoch's re-send. Sent again, as + /// signed, each poll until every node accepts it. + Partly(SignedProposerPreferences), + /// Every node accepted this message, last in the re-send round `round` + /// (see [`round_of`]). Sent again, as signed, in each later round, for a + /// node that restarted since and lost it. + Everywhere { + signed: SignedProposerPreferences, + round: Epoch, + }, +} + +/// The re-send round a poll at `current_slot` belongs to: the epoch of the +/// slot after it. An epoch's round thus starts at the poll of the last slot +/// before it, while every one of its slots is still ahead. +fn round_of(current_slot: Slot) -> Epoch { + compute_epoch_at_slot(current_slot + 1) +} + +impl Sent { + /// The message some node accepted, if one did. + fn accepted(&self) -> Option<&SignedProposerPreferences> { + match self { + Sent::Refused => None, + Sent::Partly(signed) | Sent::Everywhere { signed, .. } => Some(signed), + } + } +} + +/// Where a preference stood before this poll sent it. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum Before { + /// Never sent: signed this poll. + Unsent, + /// Sent, and no node accepted it: signed again this poll. + Refused, + /// Some node accepted it, and not every node is known to hold it. + Partly, + /// Every node accepted it in an earlier round: this is this round's + /// re-send (see [`round_of`]). + Everywhere, +} + +/// What one send came to, summed over a target epoch's sends. +#[derive(Debug, Default, Clone, Copy)] +struct Tally { + /// A node accepted it for the first time. + published: bool, + /// It went from every node or none to some: the poll warns once for all. + newly_partial: bool, +} /// Signs and publishes proposer preferences. pub struct PreferencesService { beacon_node: Arc, context: Arc, settings: Arc, - /// `(proposal_slot, dependent_root)` pairs every node accepted, so a - /// preference is signed once per dependent root and sent until all of - /// them hold it. A reorg that moves the root sends the slot again; pruned - /// below the current epoch. - published: Mutex>, - /// Preferences some node accepted and another did not, by - /// `(proposal_slot, dependent_root)`, as signed: each poll sends this - /// same message to every node again until all of them accepted it, when - /// it moves to `published`. Pruned with `published`. - partly_published: Mutex>, + /// Where each `(proposal_slot, dependent_root)` stands, so a preference + /// is signed once per dependent root, sent until every node holds it, and + /// sent again once an epoch after. A reorg that moves the root starts a + /// new entry. Pruned below the current epoch. + sent: Mutex>, /// Epochs whose missing fee recipients were already warned about, so the /// warning is once per epoch rather than once per slot. Pruned like - /// `published`. + /// `sent`. no_fee_warned: Mutex>, - /// `(proposal_slot, dependent_root)` entries whose refusal was already - /// warned about, so a persistently refused one warns once rather than every - /// slot until its slot passes. Pruned with `published`. - refusal_warned: Mutex>, /// Epochs whose failed duties fetch was already warned about, for the same /// reason: a syncing node, or one without v2, fails every slot. fetch_warned: Mutex>, /// One poll at a time: a slow node must not stack polls up. in_flight: InFlight, + /// The wall clock, so a preference whose slot started while the poll + /// waited on a node is not sent, nor reported as refused when its slot + /// started in flight. `None` goes by the poll's slot alone. + clock: Option, } -/// Lock `mutex`, recovering a poisoned one: every write here is one set -/// operation, so the data behind it is never half-changed. +/// Lock `mutex`, recovering a poisoned one: every write here is one map or +/// set operation, so the data behind it is never half-changed. fn lock(mutex: &Mutex) -> MutexGuard<'_, T> { mutex .lock() .unwrap_or_else(std::sync::PoisonError::into_inner) } +impl PreferencesService { + /// The fee recipients of the preferences some node accepted for `slot`, + /// one per dependent root signed for. + /// + /// From gloas a node builds with the preference it holds for the slot, so + /// a block at `slot` may rightly pay one of these rather than the + /// validator's current address: the preference names the address in + /// force when it was signed, and cannot be replaced for its root. + pub fn accepted_fee_recipients(&self, slot: Slot) -> Vec { + lock(&self.sent) + .iter() + .filter(|((held, _), _)| *held == slot) + .filter_map(|(_, sent)| sent.accepted()) + .map(|signed| signed.message.fee_recipient) + .collect() + } +} + impl PreferencesService { pub fn new( beacon_node: Arc, @@ -142,15 +237,21 @@ impl PreferencesService { beacon_node, context, settings, - published: Mutex::new(HashSet::new()), - partly_published: Mutex::new(HashMap::new()), + sent: Mutex::new(HashMap::new()), no_fee_warned: Mutex::new(HashSet::new()), - refusal_warned: Mutex::new(HashSet::new()), fetch_warned: Mutex::new(HashSet::new()), in_flight: InFlight::default(), + clock: None, } } + /// Give the service the slot clock, so a poll that waited on a node skips + /// the slots that started meanwhile rather than having them refused. + pub fn with_clock(mut self, clock: SlotClock) -> Self { + self.clock = Some(clock); + self + } + /// Publish what this client's validators still owe for the current and the /// next epoch, as of `current_slot`. Called once per slot, off the slot /// loop, so a slow node cannot delay the next slot's duties. @@ -160,19 +261,39 @@ impl PreferencesService { let Some(_running) = self.in_flight.enter() else { return; }; + self.publish(current_slot, &store).await; + + let epoch = compute_epoch_at_slot(current_slot); + let epoch_start = compute_start_slot_at_epoch(epoch); + lock(&self.sent).retain(|(slot, _), _| *slot >= epoch_start); + lock(&self.no_fee_warned).retain(|warned| *warned >= epoch); + lock(&self.fetch_warned).retain(|warned| *warned >= epoch); + } + /// Both target epochs, at once, so a node that does not answer costs the + /// poll one wait rather than one per epoch. + async fn publish(&self, current_slot: Slot, store: &Arc>) { let epoch = compute_epoch_at_slot(current_slot); + // Before the store or any node is asked: until the epoch before + // gloas, and with no keys loaded, nothing is owed. + let targets: Vec = [epoch, epoch + 1] + .into_iter() + .filter(|target| self.context.config.fork_at_epoch(*target) >= ForkName::Gloas) + .collect(); + if targets.is_empty() { + return; + } // Snapshotted with the guard dropped at once: nothing below awaits a // node while holding the store. let pubkeys: HashSet = store.read().await.pubkeys().into_iter().collect(); + if pubkeys.is_empty() { + return; + } - for target in [epoch, epoch + 1] { - if self.context.config.fork_at_epoch(target) < ForkName::Gloas { - continue; - } - let published = self - .publish_epoch(current_slot, target, &pubkeys, &store) - .await; + let sends = targets + .iter() + .map(|target| self.publish_epoch(current_slot, *target, &pubkeys, store)); + for (target, published) in targets.iter().zip(join_all(sends).await) { if published > 0 { crate::metrics::inc_proposer_preferences_published(published); info!( @@ -182,20 +303,23 @@ impl PreferencesService { ); } } + } - let epoch_start = epoch * preset::SLOTS_PER_EPOCH; - lock(&self.published).retain(|(slot, _)| *slot >= epoch_start); - lock(&self.partly_published).retain(|(slot, _), _| *slot >= epoch_start); - lock(&self.refusal_warned).retain(|(slot, _)| *slot >= epoch_start); - lock(&self.no_fee_warned).retain(|warned| *warned >= epoch); - lock(&self.fetch_warned).retain(|warned| *warned >= epoch); + /// Whether `slot` has started: by the clock when there is one, which a + /// poll that waited on a node needs, else by the poll's own slot. + fn started(&self, slot: Slot, current_slot: Slot) -> bool { + slot <= current_slot + || self + .clock + .as_ref() + .and_then(SlotClock::now) + .is_some_and(|now| now >= slot) } /// One target epoch: fetch its v2 duties, sign what is still owed, and - /// send it along with what some node still lacks. Returns how many + /// send it along with what some node may still lack. Returns how many /// preferences a node accepted for the first time, which is all the - /// published count takes: a re-send to the nodes that missed one is not - /// counted again. + /// published count takes: a re-send is not counted again. async fn publish_epoch( &self, current_slot: Slot, @@ -220,13 +344,14 @@ impl PreferencesService { } }; let dependent_root = duties.dependent_root; + let round = round_of(current_slot); let mut owed = Vec::new(); let mut resend = Vec::new(); let mut no_fee_recipient = 0usize; for duty in within_epoch(target, duties.duties) { // Started slots are skipped: gossip would IGNORE them. - if duty.slot <= current_slot { + if self.started(duty.slot, current_slot) { continue; } let Ok(pubkey) = parse_pubkey(&duty.pubkey) else { @@ -236,13 +361,24 @@ impl PreferencesService { continue; } let key = (duty.slot, dependent_root); - if lock(&self.published).contains(&key) { - continue; - } - if let Some(held) = lock(&self.partly_published).get(&key) { - resend.push(held.clone()); - continue; - } + let held = lock(&self.sent).get(&key).cloned(); + let before = match held { + Some(Sent::Partly(signed)) => { + resend.push((signed, Before::Partly)); + continue; + } + Some(Sent::Everywhere { + signed, + round: held, + }) => { + if held < round { + resend.push((signed, Before::Everywhere)); + } + continue; + } + Some(Sent::Refused) => Before::Refused, + None => Before::Unsent, + }; // One read for both, so a keymanager write between two reads // cannot pair an old address with a new gas limit. let Some((fee_recipient, target_gas_limit)) = @@ -251,16 +387,14 @@ impl PreferencesService { no_fee_recipient += 1; continue; }; - owed.push(( - pubkey, - ProposerPreferences { - dependent_root, - proposal_slot: duty.slot, - validator_index: duty.validator_index, - fee_recipient, - target_gas_limit, - }, - )); + let message = ProposerPreferences { + dependent_root, + proposal_slot: duty.slot, + validator_index: duty.validator_index, + fee_recipient, + target_gas_limit, + }; + owed.push((pubkey, message, before)); } if no_fee_recipient > 0 && lock(&self.no_fee_warned).insert(target) { @@ -276,16 +410,22 @@ impl PreferencesService { // The target epoch's fork, which is what the submitted objects belong // to (always gloas here). let fork = self.context.config.fork_at_epoch(target); - for entry in resend { - self.submit(fork, target, entry, true).await; - } - let mut first_accepted = 0usize; - for entry in signed { - if self.submit(fork, target, entry, false).await { - first_accepted += 1; - } + let sends = resend + .into_iter() + .chain(signed) + .map(|(entry, before)| self.submit(fork, target, current_slot, entry, before)); + let tallies = join_all(sends).await; + + let newly_partial = tallies.iter().filter(|tally| tally.newly_partial).count(); + if newly_partial > 0 { + warn!( + epoch = target, + count = newly_partial, + "Not every beacon node took these proposer preferences, and a node gossips only \ + what it took; sending them to every node each slot until all of them do" + ); } - first_accepted + tallies.iter().filter(|tally| tally.published).count() } /// Sign each of `owed`, skipping with a warning any it cannot sign, such as @@ -300,23 +440,25 @@ impl PreferencesService { async fn sign( &self, store: &Arc>, - owed: Vec<(BlsPubkey, ProposerPreferences)>, - ) -> Vec { + owed: Vec<(BlsPubkey, ProposerPreferences, Before)>, + ) -> Vec<(SignedProposerPreferences, Before)> { if owed.is_empty() { return Vec::new(); } let store = store.read().await; let mut signed = Vec::with_capacity(owed.len()); - for (pubkey, message) in owed { + for (pubkey, message, before) in owed { match self .context .sign_proposer_preferences(&store, &pubkey, &message) { - Ok(signature) => signed.push(SignedProposerPreferences { message, signature }), + Ok(signature) => { + signed.push((SignedProposerPreferences { message, signature }, before)); + } Err(err) => { warn!( - pubkey = %encode_hex(&pubkey.0), slot = message.proposal_slot, + pubkey = %encode_hex(&pubkey.0), %err, "Could not sign proposer preferences; skipping them" ); @@ -327,73 +469,118 @@ impl PreferencesService { } /// Send one preference to every node and record what came of it. - /// `resent` marks one some node already accepted, taken from - /// `partly_published`. Returns whether a node accepted it for the first - /// time. + /// `before` is where it stood before this poll. + /// + /// The failures counter takes every send some node did not take, re-sends + /// included, unless the slot started before the answer came. async fn submit( &self, fork: ForkName, target: Epoch, + current_slot: Slot, entry: SignedProposerPreferences, - resent: bool, - ) -> bool { - // One request per preference: a node answers 400 for the whole call - // when any entry fails, and the one it refuses (for a `(slot, root)` it - // holds with other content, say) would otherwise keep the others from - // ever being marked published. + before: Before, + ) -> Tally { + // One request per preference: see the module doc. let key = (entry.message.proposal_slot, entry.message.dependent_root); + let validator = entry.message.validator_index; let answer = self .beacon_node .submit_proposer_preferences(fork, std::slice::from_ref(&entry)) .await; + let first = matches!(before, Before::Unsent | Before::Refused); + let late = self.started(key.0, current_slot); match answer { Ok(Delivery::Everywhere) => { - lock(&self.partly_published).remove(&key); - lock(&self.published).insert(key); - !resent + let held = Sent::Everywhere { + signed: entry, + round: round_of(current_slot), + }; + lock(&self.sent).insert(key, held); + Tally { + published: first, + newly_partial: false, + } } Ok(Delivery::Partly) => { // Kept as signed, so the nodes that missed it get this very // message next slot. The fallback logged each refusal at debug. - lock(&self.partly_published).insert(key, entry); - !resent + lock(&self.sent).insert(key, Sent::Partly(entry)); + if !late { + crate::metrics::inc_proposer_preferences_failures(); + } + Tally { + published: first, + newly_partial: !late && before != Before::Partly, + } } - Err(err) if resent => { - // Not a failure in the warning's or the metric's sense: a node - // already gossiped it. It stays kept for the next slot. + Err(err) if late => { debug!( epoch = target, slot = key.0, - validator = entry.message.validator_index, + validator, %err, - "No beacon node accepted a proposer preference sent again" + "A proposer preference's slot started before a beacon node took it" ); - false + Tally::default() } Err(err) => { - // A refused entry is tried again every slot until its slot - // passes, so only its first refusal is a warning. - if lock(&self.refusal_warned).insert(key) { - warn!( - epoch = target, - slot = key.0, - validator = entry.message.validator_index, - %err, - "No beacon node accepted a proposer preference; builders' bids for \ - that slot are ignored, and from gloas a node may not know the fee \ - recipient" - ); - } else { - debug!( - epoch = target, - slot = key.0, - validator = entry.message.validator_index, - %err, - "A proposer preference was refused again" - ); - } crate::metrics::inc_proposer_preferences_failures(); - false + match before { + Before::Unsent => { + // A refused entry is tried again every slot until its + // slot passes, so only its first refusal is a warning. + warn!( + epoch = target, + slot = key.0, + validator, + %err, + "No beacon node accepted a proposer preference; builders' bids \ + for that slot are ignored, and from gloas a node may not know \ + the fee recipient" + ); + lock(&self.sent).insert(key, Sent::Refused); + Tally::default() + } + Before::Refused => { + debug!( + epoch = target, + slot = key.0, + validator, + %err, + "A proposer preference was refused again" + ); + Tally::default() + } + Before::Partly => { + // A node already took it; kept for the next slot. + debug!( + epoch = target, + slot = key.0, + validator, + %err, + "No beacon node took a proposer preference sent again" + ); + Tally::default() + } + Before::Everywhere => { + // Every node took it before, and none takes the + // epoch's re-send: restarted, and not ready yet. Sent + // each slot from now on, as one only some hold. + debug!( + epoch = target, + slot = key.0, + validator, + %err, + "No beacon node took this epoch's re-send of a proposer preference" + ); + lock(&self.sent).insert(key, Sent::Partly(entry)); + Tally { + published: false, + newly_partial: true, + } + } + } } } } @@ -434,6 +621,7 @@ mod tests { use crate::test_support::{settings, store_with_keys}; use ethlambda_types::beacon::config::Config; use ethlambda_types::beacon::constants::DOMAIN_PROPOSER_PREFERENCES; + use ethlambda_types::beacon::preset; use ethlambda_types::beacon::primitives::{H160, HashTreeRoot as _}; use ethlambda_types::beacon::signing::{compute_domain, compute_signing_root}; @@ -449,6 +637,21 @@ mod tests { }) } + /// Where each kept `(slot, dependent_root)` stands, by name. + fn stages(service: &PreferencesService) -> HashMap<(Slot, Root), &'static str> { + lock(&service.sent) + .iter() + .map(|(key, sent)| { + let stage = match sent { + Sent::Refused => "refused", + Sent::Partly(_) => "partly", + Sent::Everywhere { .. } => "everywhere", + }; + (*key, stage) + }) + .collect() + } + fn duty(pubkey: &BlsPubkey, validator_index: u64, slot: Slot) -> ProposerDutyDto { ProposerDutyDto { pubkey: encode_hex(&pubkey.0), @@ -720,8 +923,11 @@ mod tests { service.clone().poll(current + 1, store.clone()).await; assert_eq!(submitted(&node).len(), 1); assert_eq!( - *lock(&service.refusal_warned), - HashSet::from([(refused, Root::repeat_byte(0x11))]), + stages(&service), + HashMap::from([ + ((refused, Root::repeat_byte(0x11)), "refused"), + ((accepted, Root::repeat_byte(0x11)), "everywhere"), + ]), "two refusals of one entry leave one warn-path record" ); @@ -876,8 +1082,11 @@ mod tests { ]; assert_eq!(sent, expected); assert_eq!( - *lock(&service.published), - expected.into_iter().collect::>() + stages(&service), + expected + .into_iter() + .map(|key| (key, "everywhere")) + .collect::>() ); } @@ -967,17 +1176,18 @@ mod tests { assert_eq!(counted, 1, "published once a node took it"); assert_eq!(held(0).len(), 1); assert!(held(1).is_empty()); - assert!(lock(&service.published).is_empty(), "not every node has it"); + let partly = HashMap::from([((slot, root), "partly")]); + assert_eq!(stages(&service), partly, "not every node has it"); - // Still refused: sent to both again, not counted again, and no warning - // record, since a node holds it. + // Still refused: sent to both again, not counted again, and not a + // refusal, since a node holds it. let counted = service .publish_epoch(current + 1, GLOAS_EPOCH, &snapshot, &store) .await; assert_eq!(counted, 0); assert_eq!(held(0).len(), 2); assert!(held(1).is_empty()); - assert!(lock(&service.refusal_warned).is_empty()); + assert_eq!(stages(&service), partly); fallback.nodes()[1] .refused_preference_slots @@ -990,8 +1200,10 @@ mod tests { assert_eq!(counted, 0, "the accepting node's copy is not counted twice"); let first = held(0)[0].clone(); assert_eq!(held(1), vec![first], "the message as first signed"); - assert_eq!(*lock(&service.published), HashSet::from([(slot, root)])); - assert!(lock(&service.partly_published).is_empty()); + assert_eq!( + stages(&service), + HashMap::from([((slot, root), "everywhere")]) + ); // Every node has it: nothing more is sent. service @@ -1040,8 +1252,286 @@ mod tests { assert_eq!(sent.len(), 1); assert_eq!(sent[0].1[0].message.validator_index, 4); assert_eq!( - *lock(&service.published), - HashSet::from([(kept, Root::repeat_byte(0x11))]) + stages(&service), + HashMap::from([((kept, Root::repeat_byte(0x11)), "everywhere")]) + ); + } + + /// A node that restarts loses what it accepted. Each preference every node + /// took is sent again, as first signed, once a round: at the poll of an + /// epoch's last slot, so the next epoch's first slot is covered too. A + /// re-send is not counted as published again. + #[tokio::test] + async fn every_kept_preference_is_sent_again_once_an_epoch() { + let (store, pubkeys) = store_with_keys(1); + let snapshot: HashSet = pubkeys.iter().copied().collect(); + let store = Arc::new(RwLock::new(store)); + let first_slot = slot_in(GLOAS_EPOCH + 1, 0); + let later_slot = slot_in(GLOAS_EPOCH + 1, 4); + let root = Root::repeat_byte(0x22); + let node = MockBeaconNode::new().with_proposers_v2( + GLOAS_EPOCH + 1, + root, + vec![ + duty(&pubkeys[0], 3, first_slot), + duty(&pubkeys[0], 3, later_slot), + ], + ); + let settings = settings(Some(FEE_RECIPIENT)); + let (service, node) = service(node, settings.clone()); + let publish = |current: Slot| { + let service = service.clone(); + let snapshot = snapshot.clone(); + let store = store.clone(); + async move { + service + .publish_epoch(current, GLOAS_EPOCH + 1, &snapshot, &store) + .await + } + }; + let last_of_epoch = slot_in(GLOAS_EPOCH, preset::SLOTS_PER_EPOCH - 1); + let sent_slots = |from: usize| -> Vec { + let mut slots: Vec = submitted(&node)[from..] + .iter() + .map(|(_, entries)| entries[0].message.proposal_slot) + .collect(); + slots.sort(); + slots + }; + + // Published the epoch before their own, and not again before its end. + assert_eq!(publish(slot_in(GLOAS_EPOCH, 1)).await, 2); + publish(last_of_epoch - 1).await; + assert_eq!(submitted(&node).len(), 2); + assert_eq!( + service.accepted_fee_recipients(later_slot), + vec![FEE_RECIPIENT] + ); + + // The address changes; the round's re-send still carries the one + // signed, for both slots, the next epoch's first included. + settings.set_fee_recipient(&pubkeys[0], H160([0xcd; 20])); + assert_eq!( + publish(last_of_epoch).await, + 0, + "a re-send is not published again" + ); + assert_eq!(sent_slots(2), vec![first_slot, later_slot]); + let sent = submitted(&node); + let mut first: Vec<_> = sent[..2].iter().map(|(_, entries)| &entries[0]).collect(); + let mut again: Vec<_> = sent[2..].iter().map(|(_, entries)| &entries[0]).collect(); + first.sort_by_key(|entry| entry.message.proposal_slot); + again.sort_by_key(|entry| entry.message.proposal_slot); + assert_eq!(again, first, "the messages as first signed"); + assert_eq!( + service.accepted_fee_recipients(later_slot), + vec![FEE_RECIPIENT] + ); + + // Once a round: the next epoch's polls send nothing more. + publish(first_slot).await; + publish(first_slot + 1).await; + assert_eq!(submitted(&node).len(), 4); + } + + /// A round's re-send no node takes (every node restarted and is not ready + /// yet) is sent each slot from then on, until one does. + #[tokio::test] + async fn a_re_send_no_node_takes_is_sent_each_slot() { + let (store, pubkeys) = store_with_keys(1); + let snapshot: HashSet = pubkeys.iter().copied().collect(); + let store = Arc::new(RwLock::new(store)); + let slot = slot_in(GLOAS_EPOCH + 1, 4); + let root = Root::repeat_byte(0x22); + let node = MockBeaconNode::new().with_proposers_v2( + GLOAS_EPOCH + 1, + root, + vec![duty(&pubkeys[0], 3, slot)], ); + let (service, node) = service(node, settings(Some(FEE_RECIPIENT))); + let publish = |current: Slot| { + let service = service.clone(); + let snapshot = snapshot.clone(); + let store = store.clone(); + async move { + service + .publish_epoch(current, GLOAS_EPOCH + 1, &snapshot, &store) + .await + } + }; + + publish(slot_in(GLOAS_EPOCH, 1)).await; + node.failures + .lock() + .expect("lock") + .insert("submit_proposer_preferences", "syncing".to_string()); + publish(slot_in(GLOAS_EPOCH, preset::SLOTS_PER_EPOCH - 1)).await; + let key = (slot, root); + assert_eq!(stages(&service), HashMap::from([(key, "partly")])); + assert_eq!(service.accepted_fee_recipients(slot), vec![FEE_RECIPIENT]); + + node.failures.lock().expect("lock").clear(); + publish(slot_in(GLOAS_EPOCH + 1, 0)).await; + assert_eq!(submitted(&node).len(), 2, "sent again the next slot"); + assert_eq!(stages(&service), HashMap::from([(key, "everywhere")])); + } + + /// The case a restart makes with two nodes: both took a preference, and + /// one refuses the round's re-send (restarted, not synced yet). It drops + /// to one only some nodes hold, is not counted again, and goes to both + /// nodes each slot until the refusing one takes it. + #[tokio::test] + async fn a_re_send_one_of_two_nodes_refuses_is_sent_each_slot() { + use crate::beacon_node::fallback::FallbackBeaconNode; + + let (store, pubkeys) = store_with_keys(1); + let snapshot: HashSet = pubkeys.iter().copied().collect(); + let store = Arc::new(RwLock::new(store)); + let slot = slot_in(GLOAS_EPOCH + 1, 4); + let root = Root::repeat_byte(0x22); + let node = || { + MockBeaconNode::new().with_proposers_v2( + GLOAS_EPOCH + 1, + root, + vec![duty(&pubkeys[0], 3, slot)], + ) + }; + let fallback = Arc::new(FallbackBeaconNode::new(vec![node(), node()])); + let service = + PreferencesService::new(fallback.clone(), context(), settings(Some(FEE_RECIPIENT))); + let held = |position: usize| { + fallback.nodes()[position] + .submitted_preferences + .lock() + .expect("lock") + .len() + }; + let refusing = |refuse: bool| { + let mut slots = fallback.nodes()[1] + .refused_preference_slots + .lock() + .expect("lock"); + slots.clear(); + if refuse { + slots.push(slot); + } + }; + let key = (slot, root); + + let counted = service + .publish_epoch(slot_in(GLOAS_EPOCH, 1), GLOAS_EPOCH + 1, &snapshot, &store) + .await; + assert_eq!(counted, 1); + assert_eq!(stages(&service), HashMap::from([(key, "everywhere")])); + + refusing(true); + let last_of_epoch = slot_in(GLOAS_EPOCH, preset::SLOTS_PER_EPOCH - 1); + let counted = service + .publish_epoch(last_of_epoch, GLOAS_EPOCH + 1, &snapshot, &store) + .await; + assert_eq!(counted, 0, "a re-send is not published again"); + assert_eq!((held(0), held(1)), (2, 1)); + assert_eq!(stages(&service), HashMap::from([(key, "partly")])); + + refusing(false); + let counted = service + .publish_epoch( + slot_in(GLOAS_EPOCH + 1, 0), + GLOAS_EPOCH + 1, + &snapshot, + &store, + ) + .await; + assert_eq!(counted, 0); + assert_eq!((held(0), held(1)), (3, 2), "sent to both the next slot"); + assert_eq!(stages(&service), HashMap::from([(key, "everywhere")])); + } + + /// A poll that started late, its slot behind the clock's, does not send a + /// preference whose slot has started since. + #[tokio::test] + async fn a_slot_the_clock_says_started_is_not_sent() { + use std::time::{SystemTime, UNIX_EPOCH}; + + let (store, pubkeys) = store_with_keys(1); + let store = Arc::new(RwLock::new(store)); + let started = slot_in(GLOAS_EPOCH, 3); + let ahead = slot_in(GLOAS_EPOCH, 10); + let node = MockBeaconNode::new().with_proposers_v2( + GLOAS_EPOCH, + Root::repeat_byte(0x11), + vec![duty(&pubkeys[0], 3, started), duty(&pubkeys[0], 3, ahead)], + ); + // One second into slot 5 of the gloas epoch. + let config = context().config.clone(); + let now = SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("after 1970") + .as_secs(); + let clock_slot = slot_in(GLOAS_EPOCH, 5); + let genesis = now - clock_slot * config.slot_duration_ms / 1000 - 1; + let clock = SlotClock::from_config(genesis, &config); + let node = Arc::new(node); + let service = Arc::new( + PreferencesService::new(node.clone(), context(), settings(Some(FEE_RECIPIENT))) + .with_clock(clock), + ); + + service.poll(slot_in(GLOAS_EPOCH, 1), store).await; + + let sent = submitted(&node); + assert_eq!(sent.len(), 1); + assert_eq!(sent[0].1[0].message.proposal_slot, ahead); + } + + /// No keys loaded: no duties are fetched and nothing is signed. + #[tokio::test] + async fn with_no_keys_loaded_no_duties_are_fetched() { + let (_, pubkeys) = store_with_keys(1); + let store = Arc::new(RwLock::new(ValidatorStore::new())); + let node = MockBeaconNode::new().with_proposers_v2( + GLOAS_EPOCH, + Root::repeat_byte(0x11), + vec![duty(&pubkeys[0], 3, slot_in(GLOAS_EPOCH, 10))], + ); + let (service, node) = service(node, settings(Some(FEE_RECIPIENT))); + + service.poll(slot_in(GLOAS_EPOCH, 1), store).await; + + assert_eq!(node.proposer_duties_v2_call_count(), 0); + assert!(submitted(&node).is_empty()); + } + + /// Every preference of a poll, both target epochs', is sent at once: a + /// node that takes the whole request timeout to answer costs the poll one + /// timeout, not one per preference. + #[tokio::test(start_paused = true)] + async fn a_slow_node_costs_a_poll_one_wait() { + let (store, pubkeys) = store_with_keys(1); + let store = Arc::new(RwLock::new(store)); + let wait = std::time::Duration::from_secs(8); + let node = MockBeaconNode::new() + .with_proposers_v2( + GLOAS_EPOCH, + Root::repeat_byte(0x11), + (10..13) + .map(|offset| duty(&pubkeys[0], 3, slot_in(GLOAS_EPOCH, offset))) + .collect(), + ) + .with_proposers_v2( + GLOAS_EPOCH + 1, + Root::repeat_byte(0x22), + (4..6) + .map(|offset| duty(&pubkeys[0], 3, slot_in(GLOAS_EPOCH + 1, offset))) + .collect(), + ) + .with_preference_delay(wait); + let (service, node) = service(node, settings(Some(FEE_RECIPIENT))); + + let began = tokio::time::Instant::now(); + service.poll(slot_in(GLOAS_EPOCH, 1), store).await; + + assert_eq!(submitted(&node).len(), 5); + assert_eq!(began.elapsed(), wait, "five sends, one wait"); } } diff --git a/crates/validator/src/proposal.rs b/crates/validator/src/proposal.rs index 3a7f0931d..01ac62888 100644 --- a/crates/validator/src/proposal.rs +++ b/crates/validator/src/proposal.rs @@ -42,6 +42,10 @@ //! //! The envelope is not slashable, so slashing protection is untouched by it. //! +//! The node pays the fee recipient of the proposer preference it holds for +//! the slot, which names the address in force when it was signed. The check +//! before signing therefore accepts that address as well as the current one. +//! //! # Blinded blocks //! //! With builder proposals on, the node may answer with a blinded block: the @@ -88,6 +92,7 @@ use crate::beacon_node::dto::{ProposerDutyDto, encode_hex, parse_pubkey}; use crate::beacon_node::{BeaconNodeApi, BlockRequest, Published, validate_produced_block}; use crate::error::{Error, Result}; use crate::keys::ValidatorStore; +use crate::preferences::PreferencesService; use crate::proposer_settings::ProposerSettings; use crate::signing::SigningContext; use crate::slashing_protection::{Refusal, SharedProtection}; @@ -116,6 +121,10 @@ pub struct ProposalService { /// Where a slot starts, so a blinded publish that outlives the proposal /// can observe its own publication delay. `None` observes nothing. clock: Option, + /// The proposer preferences a node accepted, whose fee recipient a gloas + /// block may rightly pay instead of the current one. `None` compares + /// against the current one alone. + preferences: Option>>, } impl ProposalService { @@ -134,9 +143,18 @@ impl ProposalService { builder_boost_factor, last_blinded_slot: std::sync::Mutex::new(None), clock: None, + preferences: None, } } + /// Give the service the proposer preferences this client published, so + /// the fee recipient check accepts the address a gloas block's node was + /// given in them. + pub fn with_preferences(mut self, preferences: Arc>) -> Self { + self.preferences = Some(preferences); + self + } + /// Give the service the slot clock, so a blinded block's publication delay /// is observed by the task that publishes it (which may outlive /// [`Self::propose`]). @@ -187,33 +205,55 @@ impl ProposalService { /// never happen, and when it does it is a misconfigured or untrustworthy /// beacon node paying someone else, every time this validator proposes. /// - /// With no configured address there is nothing to compare against and this - /// says nothing, which is the same silence as a matching one; the startup - /// warning is where that case is reported. + /// With no configured address and no published preference there is + /// nothing to compare against and this says nothing, which is the same + /// silence as a matching one; the startup warning is where that case is + /// reported. fn check_fee_recipient( produced: &crate::beacon_node::block_contents::ProducedBlock, - expected: Option, + current: Option, + preferred: &[ExecutionAddress], slot: Slot, ) { - let Some(expected) = expected else { - return; - }; // The payload's before gloas, the bid's from it on: the same address // the execution layer is told to pay, read where the block keeps it. let actual = produced.fee_recipient(); - if actual != expected { - tracing::error!( - %slot, - expected = %crate::beacon_node::dto::encode_hex(&expected.0), - actual = %crate::beacon_node::dto::encode_hex(&actual.0), - "Block pays its execution-layer rewards to an address this client did not ask \ - for; signing it anyway, since refusing would also forfeit the consensus reward \ - and cost the network a slot. Check this beacon node's proposer preparation. A \ - fee recipient changed through the keymanager API this epoch reaches the node \ - only at the next one." - ); - crate::metrics::inc_fee_recipient_mismatches(); + if Self::pays_this_validator(actual, current, preferred) != Some(false) { + return; + } + let expected: Vec = current + .iter() + .chain(preferred) + .map(|address| crate::beacon_node::dto::encode_hex(&address.0)) + .collect(); + tracing::error!( + %slot, + expected = %expected.join(","), + actual = %crate::beacon_node::dto::encode_hex(&actual.0), + "Block pays its execution-layer rewards to an address this client did not ask for; \ + signing it anyway, since refusing would also forfeit the consensus reward and cost \ + the network a slot. Check this beacon node's proposer preparation. A fee recipient \ + changed through the keymanager API reaches the node at the next epoch boundary; \ + from gloas, a proposer preference published before the change (by this process \ + before a restart, say) keeps the old address until the next dependent root." + ); + crate::metrics::inc_fee_recipient_mismatches(); + } + + /// Whether `actual` is an address this validator asked to be paid at: its + /// `current` one, or one of the proposer preferences a node accepted for + /// the slot (`preferred`), which from gloas a node builds with in + /// preference to the current address, and which name the address in force + /// when they were signed. `None` when there is nothing to compare against. + fn pays_this_validator( + actual: ExecutionAddress, + current: Option, + preferred: &[ExecutionAddress], + ) -> Option { + if current.is_none() && preferred.is_empty() { + return None; } + Some(current == Some(actual) || preferred.contains(&actual)) } /// Produce, sign and publish the block for `slot`. @@ -309,7 +349,12 @@ impl ProposalService { } if Self::compares_fee_recipient(&produced) { - Self::check_fee_recipient(&produced, fee_recipient, slot); + let preferred = self + .preferences + .as_ref() + .map(|preferences| preferences.accepted_fee_recipients(slot)) + .unwrap_or_default(); + Self::check_fee_recipient(&produced, fee_recipient, &preferred, slot); } else { tracing::debug!( %slot, @@ -1048,6 +1093,25 @@ mod tests { )); } + /// The current address and every accepted preference's are this + /// validator's; anything else is a mismatch, and with neither configured + /// there is nothing to compare. + #[test] + fn a_block_may_pay_the_current_address_or_a_published_preferences() { + let pays = ProposalService::::pays_this_validator; + let current = H160([0x01; 20]); + let preferred = H160([0x02; 20]); + let stranger = H160([0x03; 20]); + + assert_eq!(pays(current, Some(current), &[]), Some(true)); + assert_eq!(pays(preferred, Some(current), &[preferred]), Some(true)); + assert_eq!(pays(current, Some(current), &[preferred]), Some(true)); + assert_eq!(pays(stranger, Some(current), &[preferred]), Some(false)); + assert_eq!(pays(stranger, Some(current), &[]), Some(false)); + assert_eq!(pays(stranger, None, &[preferred]), Some(false)); + assert_eq!(pays(stranger, None, &[]), None); + } + /// A node that does not confirm the publish may still have handed the block /// to its relay, so it is not a missed proposal, and the slot is spent: a /// second block would be an equivocation. diff --git a/crates/validator/src/proposer_settings.rs b/crates/validator/src/proposer_settings.rs index 2d73f4397..ff4480f69 100644 --- a/crates/validator/src/proposer_settings.rs +++ b/crates/validator/src/proposer_settings.rs @@ -20,6 +20,12 @@ //! change is registered at the next epoch boundary. A block proposed before //! then pays the address the node was last told, which the check before //! signing reports as a mismatch. +//! +//! From gloas a node pays the address in the proposer preferences it holds for +//! the slot instead, and gas limit likewise, and a preference cannot be +//! replaced for its dependent root: a change reaches gloas blocks from the next +//! dependent root on. The check before signing accepts the address of a +//! preference a node accepted. use std::collections::HashMap; use std::sync::atomic::{AtomicU64, Ordering}; diff --git a/docs/cli.md b/docs/cli.md index 2a1ab647f..7da8b8633 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -337,8 +337,14 @@ for a slot with none. It reads those duties from the v2 proposer duties endpoint, whose dependent root the preferences must carry, and publishes each `(slot, dependent root)` until every node has accepted it, sending the message as first signed to all of them again each slot until then, and again after a -reorg changes the root. Duties outside their epoch, and all but the first for -a slot, are dropped before anything is signed. A +reorg changes the root. After that it sends each one, as first signed, once +more every epoch while its slot is ahead, at the epoch's last slot, since a +node that restarts loses what it accepted; a node restarted mid-epoch thus +lacks the rest of that epoch's, and has the next epoch's from its first slot. +Each preference is its own request, and a poll sends all +of them, for both epochs, at once, so a node that does not answer delays the +others by one request timeout. Duties outside their epoch, and all but the +first for a slot, are dropped before anything is signed. A validator with no fee recipient is skipped, with one warning per epoch naming the count. A preference cannot be replaced for the same slot and root, so a fee recipient changed through the keymanager API applies from the next diff --git a/docs/metrics.md b/docs/metrics.md index 789dce83c..3010d110c 100644 --- a/docs/metrics.md +++ b/docs/metrics.md @@ -751,7 +751,7 @@ and absent is not the same as zero. | `ethlambda_validator_builder_registrations_sent_total` | Counter | Builder registrations a beacon node accepted (`--builder-proposals`) | | `ethlambda_validator_builder_registration_failures_total` | Counter | Registration batches no beacon node accepted. A missed registration costs at most a builder block, never a duty | | `ethlambda_validator_proposer_preferences_published_total` | Counter | Signed proposer preferences a beacon node accepted, counted once per preference, when the first node accepts it (re-sends to nodes that missed it do not count). Published for every proposal from the epoch before gloas, whether or not `--builder-proposals` is set | -| `ethlambda_validator_proposer_preferences_failures_total` | Counter | Proposer preferences a beacon node refused or failed to take (one count per preference per attempt). The next slot retries; until a preference is accepted, builders' bids for its slot are ignored. A re-send of one some node already accepted is not counted | +| `ethlambda_validator_proposer_preferences_failures_total` | Counter | Proposer preference sends some beacon node refused or did not answer (one count per preference per attempt, re-sends included, so a node that keeps refusing shows up here even while another accepts). The next slot retries; a node that misses a preference never gossips it, and until some node accepts it, builders' bids for its slot are ignored. A send whose slot started before the nodes answered is not counted | | `ethlambda_validator_proposer_preferences_duty_fetch_failures_total` | Counter | v2 proposer-duty fetches no beacon node answered (one count per target epoch per attempt). Without the duties no preference is signed for that epoch; a syncing node, or one without the v2 endpoint, shows up here | | `ethlambda_validator_blinded_blocks_proposed_total` | Counter | Blinded blocks a beacon node handed to its builder. A subset of `blocks_proposed_total` | | `ethlambda_validator_blinded_blocks_unconfirmed_total` | Counter | Blinded blocks the beacon node did not confirm (a 5xx or no answer), which the relay may still have published. Not retried, since a second block for the slot would be an equivocation | diff --git a/docs/spec_deviations.md b/docs/spec_deviations.md index d091f3865..28ac90cdd 100644 --- a/docs/spec_deviations.md +++ b/docs/spec_deviations.md @@ -818,3 +818,9 @@ signature, whichever way the node answered. Three departures: gloas the preferences carry every proposal's fee recipient. Validators with no fee recipient are skipped (the field is required), and a preference cannot be replaced for the same slot and dependent root. +- **Re-sent once an epoch,** as signed, to every node, while its slot is ahead. + The specification says nothing about re-sending; it is here because a beacon + node holds them in memory only + and gossip does not replay them, so a node that restarts would otherwise + self-build without them, and ignore builders' bids, until the next dependent + root. Same cadence as `prepare_beacon_proposer`, which a restart loses too. From af7caebfe0cab5097b88681123d1ae68610aca89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Thu, 12 Nov 2026 18:18:13 -0300 Subject: [PATCH 15/16] feat(validator): --gas-limit for the default target gas limit From gloas the proposer preferences carry each validator's target gas limit, and produceBlockV4 builds this client's own payload toward it, with or without --builder-proposals. The default was the fixed 60M, so on a network running elsewhere (the devnet ran 200M) every default proposal pulled the limit toward 60M, and only a keymanager override, lost on restart, could change it. --gas-limit (default 60000000, under Lighthouse's flag name) sets the default every validator without an override gets: builder registrations before gloas, proposer preferences from it. Unlike Lighthouse's, it does not require --builder-proposals, since the preferences are published either way. Zero is refused. The keymanager's gas_limit endpoints serve and fall back to it. A default configuration still pulls a non-60M chain toward 60M. Reading the network config's GAS_LIMIT_SCHEDULE, which Lighthouse tries before its own 60M fallback, is left for later. --- README.md | 1 + bin/ethlambda/src/validator.rs | 37 +++++++++++++++++- crates/validator/src/lib.rs | 14 +++++-- crates/validator/src/proposer_settings.rs | 46 +++++++++++++++++++++-- docs/cli.md | 3 +- 5 files changed, 91 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index ebe06b55b..826234f22 100644 --- a/README.md +++ b/README.md @@ -369,6 +369,7 @@ the standard Beacon API, so it works against `ethlambda beacon` (run with the | `--secrets-dir` | One password file per keystore, named after the validator's public key | | `--suggested-fee-recipient` | Execution address for block rewards, `0x`-prefixed. Optional, but without it the beacon node picks an address that is not yours | | `--graffiti` | Text for proposed blocks, at most 32 bytes | +| `--gas-limit` | The gas limit validators target (default `60000000`). From gloas the beacon node builds this client's own payloads toward it, so set it on a network whose limit is elsewhere | | `--builder-proposals` | Propose builder blocks before gloas: register each validator with the beacon node's builder (`--builder` on an ethlambda node) and sign a blinded block when the builder's bid wins. Off by default | | `--enable-keymanager` | Serve the keymanager API on `--keymanager-port` (default `5062`). Off by default, since it mutates key material | diff --git a/bin/ethlambda/src/validator.rs b/bin/ethlambda/src/validator.rs index 74c67c583..2d671506f 100644 --- a/bin/ethlambda/src/validator.rs +++ b/bin/ethlambda/src/validator.rs @@ -6,7 +6,9 @@ use std::path::PathBuf; use ethlambda_types::beacon::primitives::{Bytes32, ExecutionAddress, H160}; -use ethlambda_validator::proposer_settings::{GRAFFITI_BYTES, graffiti_from_text}; +use ethlambda_validator::proposer_settings::{ + DEFAULT_GAS_LIMIT, GRAFFITI_BYTES, graffiti_from_text, +}; use ethlambda_validator::{BuilderProposals, ValidatorConfig}; /// An execution address is exactly this wide. @@ -70,6 +72,22 @@ pub(crate) struct ValidatorOptions { #[arg(long)] pub(crate) suggested_fee_recipient: Option, + /// The gas limit every validator targets, unless the keymanager API gives + /// it its own. + /// + /// Before gloas it goes into builder registrations (with + /// `--builder-proposals`). From gloas it goes into the proposer + /// preferences, published for every proposal, and the beacon node builds + /// this client's own payloads toward it too, so on a network whose gas + /// limit is not the default, set it or every proposal pulls the limit + /// toward 60M. + #[arg( + long, + default_value_t = DEFAULT_GAS_LIMIT, + value_parser = clap::value_parser!(u64).range(1..) + )] + pub(crate) gas_limit: u64, + /// Serve the keymanager API: keystores, plus each validator's own fee /// recipient, graffiti and gas limit, which are held in memory only. /// @@ -210,6 +228,7 @@ pub(crate) async fn run(options: ValidatorOptions) -> eyre::Result<()> { keymanager: options .enable_keymanager .then(|| std::net::SocketAddr::new(options.http_address, options.keymanager_port)), + gas_limit: options.gas_limit, builder, init_slashing_protection: options.init_slashing_protection, }) @@ -231,6 +250,7 @@ mod tests { metrics_port: 5064, graffiti: graffiti.to_string(), suggested_fee_recipient: None, + gas_limit: DEFAULT_GAS_LIMIT, enable_keymanager: false, builder_proposals: false, builder_boost_factor: 100, @@ -261,6 +281,21 @@ mod tests { Cli::try_parse_from(base.iter().chain(extra)).map(|cli| cli.options) } + #[test] + fn the_gas_limit_defaults_to_60m_and_takes_any_positive_value() { + assert_eq!(parse(&[]).expect("parses").gas_limit, DEFAULT_GAS_LIMIT); + let options = parse(&["--gas-limit", "200000000"]).expect("parses"); + assert_eq!(options.gas_limit, 200_000_000); + parse(&["--gas-limit", "0"]).expect_err("a zero gas limit is a mistake"); + } + + /// From gloas the gas limit is published whether or not builder proposals + /// are on, so it does not require them. + #[test] + fn the_gas_limit_does_not_need_builder_proposals() { + parse(&["--gas-limit", "30000000"]).expect("parses alone"); + } + #[test] fn builder_proposals_are_off_by_default() { assert_eq!(parse(&[]).expect("parses").builder(), None); diff --git a/crates/validator/src/lib.rs b/crates/validator/src/lib.rs index dbdfe8d61..98f084021 100644 --- a/crates/validator/src/lib.rs +++ b/crates/validator/src/lib.rs @@ -96,6 +96,11 @@ pub struct ValidatorConfig { /// Bind address for the keymanager API. `None` when `--enable-keymanager` /// was not passed, in which case it is never spawned. pub keymanager: Option, + /// The gas limit every validator targets unless the keymanager API gives + /// it its own: its builder registrations' before gloas, and its proposer + /// preferences' from gloas, which is what the node's own payload is built + /// toward. + pub gas_limit: u64, /// Builder proposals, or `None` when `--builder-proposals` is off. pub builder: Option, /// Register keys that have no slashing-protection record as never having @@ -144,10 +149,10 @@ pub async fn run(config: ValidatorConfig) -> Result<()> { preferences sign that address for the builder to pay." ); } - let settings = Arc::new(ProposerSettings::new( - config.graffiti, - config.suggested_fee_recipient, - )); + let settings = Arc::new( + ProposerSettings::new(config.graffiti, config.suggested_fee_recipient) + .with_default_gas_limit(config.gas_limit), + ); // The directory's lock first, before any key is read: held from here, // nothing another process does to the directory can get between what @@ -207,6 +212,7 @@ pub async fn run(config: ValidatorConfig) -> Result<()> { attestation_due_bps = spec.attestation_due_bps, aggregate_due_bps = spec.aggregate_due_bps, validators = store.read().await.len(), + gas_limit = config.gas_limit, "Validator client starting" ); diff --git a/crates/validator/src/proposer_settings.rs b/crates/validator/src/proposer_settings.rs index ff4480f69..54a92a65d 100644 --- a/crates/validator/src/proposer_settings.rs +++ b/crates/validator/src/proposer_settings.rs @@ -64,11 +64,16 @@ pub fn graffiti_to_text(graffiti: &Bytes32) -> String { String::from_utf8_lossy(&graffiti.0[..GRAFFITI_BYTES - padding]).into_owned() } -/// The gas limit a validator with no override reports. +/// The gas limit a validator with no override reports, unless `--gas-limit` +/// names another. /// -/// Lighthouse's and Nimbus's default. Builder registrations carry it as the -/// gas limit the builder should target, and the keymanager API serves it as the -/// specification requires. +/// Lighthouse's and Nimbus's default (Lighthouse's when the network config +/// names no gas limit schedule, which this client does not read). Builder +/// registrations carry it as the gas limit the builder should target, from +/// gloas the proposer preferences carry it as the target the node's own +/// payload is built toward too, and the keymanager API serves it as the +/// specification requires. A network whose gas limit is elsewhere needs +/// `--gas-limit`, or every proposal pulls its limit toward this one. pub const DEFAULT_GAS_LIMIT: u64 = 60_000_000; /// One validator's overrides; `None` means the default applies. @@ -126,6 +131,20 @@ impl ProposerSettings { } } + /// These settings with `gas_limit` as the gas limit of every validator + /// with no override of its own, in place of [`DEFAULT_GAS_LIMIT`]. + pub fn with_default_gas_limit(self, gas_limit: u64) -> Self { + let mut inner = self + .inner + .into_inner() + .unwrap_or_else(PoisonError::into_inner); + inner.default_gas_limit = gas_limit; + Self { + inner: RwLock::new(inner), + generation: self.generation, + } + } + /// A counter that moves whenever any override is written, cleared or /// forgotten. Equal readings mean no per-key setting changed between them. pub fn generation(&self) -> u64 { @@ -268,6 +287,25 @@ mod tests { assert_eq!(settings.gas_limit(&key(2)), DEFAULT_GAS_LIMIT); } + /// `--gas-limit` replaces the default every key without an override gets, + /// in the pair a preference signs too; an override still wins, and + /// clearing it falls back to the configured default, not the built-in one. + #[test] + fn a_configured_default_gas_limit_replaces_the_built_in_one() { + let settings = settings().with_default_gas_limit(200_000_000); + assert_eq!(settings.gas_limit(&key(1)), 200_000_000); + assert_eq!( + settings.fee_recipient_and_gas_limit(&key(1)), + Some((H160([0xaa; 20]), 200_000_000)) + ); + + settings.set_gas_limit(&key(1), 30_000_000); + assert_eq!(settings.gas_limit(&key(1)), 30_000_000); + assert_eq!(settings.gas_limit(&key(2)), 200_000_000); + settings.clear_gas_limit(&key(1)); + assert_eq!(settings.gas_limit(&key(1)), 200_000_000); + } + /// The pair a signed registration or preference carries agrees with the /// two separate reads, and is `None` exactly when there is no address. #[test] diff --git a/docs/cli.md b/docs/cli.md index 7da8b8633..a71a699ba 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -298,6 +298,7 @@ implementation's, so none of the [common flags](#common-flags) apply to it. | `--metrics-port` | `5064` | Prometheus metrics port | | `--keymanager-port` | `5062` | Keymanager API port. Only bound with `--enable-keymanager` | | `--suggested-fee-recipient` | none | Execution address to receive block rewards, `0x`-prefixed, for every validator the keymanager API gives no address of its own. Optional, and startup warns when it is absent: without it the beacon node picks an address, and it will not be yours. The zero address is accepted, as every other client accepts it, but reported at error at startup: it burns the rewards, and builder registrations and proposer preferences sign it for the builder to pay | +| `--gas-limit` | `60000000` | The gas limit every validator targets, unless the keymanager API gives it its own. Before gloas it goes into builder registrations (with `--builder-proposals`). From gloas it goes into the proposer preferences, which are published for every proposal, and the beacon node builds this client's own payloads toward it as well, so a network whose gas limit is not 60M needs it set, or every proposal pulls the limit toward 60M. Must be positive | | `--graffiti` | empty | Text for the graffiti field of proposed blocks, for every validator the keymanager API gives no graffiti of its own. At most 32 bytes as UTF-8, right-padded with zeros. Refused rather than truncated if longer. An ethlambda beacon node appends both clients' versions to it (see [`rpc.md`](rpc.md#validator-endpoints)) | | `--enable-keymanager` | off | Serve the keymanager API: keystores, plus each validator's own fee recipient, graffiti and gas limit. Off by default because it mutates key material | | `--builder-proposals` | off | Propose builder blocks: register each validator with the beacon node's builder (an ethlambda node: `--builder`) and accept a blinded block when the builder's bid wins. Registrations and blinded blocks are fulu and earlier only. From gloas the builder market works through proposer preferences, which are published with or without this flag; there it only sets the boost factor that weighs p2p bids in `produceBlockV4` | @@ -470,7 +471,7 @@ give one validator its own value in place of the command line's default: |---|---|---| | `feerecipient` | `--suggested-fee-recipient` | At the next epoch boundary, when the fee recipients are registered with the beacon node again. A block proposed before then pays the address the node was last given, and the check before signing reports the mismatch. With `--builder-proposals`, the next slot's builder registration carries it too, and an ethlambda beacon node takes it from there. From gloas, the proposer preferences carry it, from the next dependent root on | | `graffiti` | `--graffiti` | The next proposal | -| `gas_limit` | `DEFAULT_GAS_LIMIT` | Before gloas, the next slot's builder registration with `--builder-proposals`, and stored and served only without it. From gloas, the proposer preferences' target gas limit, from the next dependent root on, flag or not | +| `gas_limit` | `--gas-limit` | Before gloas, the next slot's builder registration with `--builder-proposals`, and stored and served only without it. From gloas, the proposer preferences' target gas limit, from the next dependent root on, flag or not | **These are held in memory only.** A restart puts every validator back on the defaults, so an operator driving these endpoints must set them again after one. From 320eb7b2fec9760e7978df0c3ff8fa0d9344cfb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Thu, 12 Nov 2026 18:20:27 -0300 Subject: [PATCH 16/16] feat(validator): --builder-min-bid for gloas's p2p bid floor produceBlockV4's body sent min_bid 0 while this PR turns the boost factor on, and choose_payload applies the floor to the pooled p2p bids: with --prefer-builder-proposals (u64::MAX) any 1-gwei bid beat the local payload, and with the default factor so did any bid while the local payload's value is 0. --builder-min-bid (gwei, default 0, requires --builder-proposals) is sent as that min_bid. Gloas only; produceBlockV3 has no such parameter. --- CLAUDE.md | 5 +- bin/ethlambda/src/validator.rs | 55 ++++++++++++++++++- .../rpc/src/beacon/validator_client_tests.rs | 4 ++ crates/validator/src/beacon_node/fallback.rs | 1 + crates/validator/src/beacon_node/http.rs | 36 ++++++++---- crates/validator/src/beacon_node/mod.rs | 4 ++ crates/validator/src/lib.rs | 5 ++ crates/validator/src/proposal.rs | 30 ++++++++++ docs/cli.md | 1 + docs/spec_deviations.md | 3 +- 10 files changed, 127 insertions(+), 17 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index b3cf640e9..0b30b684f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -868,7 +868,10 @@ transitions are in `ethlambda-types`, per the section above. Nothing above epoch before gloas it signs proposer preferences every slot for its proposals in the current and next epoch (`PreferencesService`, v2 duties, signed at the proposal epoch's fork version as consensus-specs v1.7.0-beta.4 and every other - client do), unconditionally, and `produceBlockV4` carries the boost factor. + client do), unconditionally, re-sending each kept one once an epoch (a node + holds them in memory only), and `produceBlockV4` carries the boost factor and + `--builder-min-bid`. `--gas-limit` sets the target gas limit the preferences + carry, which the node's own gloas payload is built toward. It keeps EIP-3076 minimal-watermark slashing protection in `--validators-dir/slashing_protection.json` (`crate::slashing_protection`), durable before every block (blinded ones included) and attestation signature; diff --git a/bin/ethlambda/src/validator.rs b/bin/ethlambda/src/validator.rs index 2d671506f..842cc88a3 100644 --- a/bin/ethlambda/src/validator.rs +++ b/bin/ethlambda/src/validator.rs @@ -124,6 +124,16 @@ pub(crate) struct ValidatorOptions { #[arg(long, default_value = "false", requires = "builder_proposals")] pub(crate) prefer_builder_proposals: bool, + /// The smallest builder bid, in gwei, a gloas block may be built on: the + /// beacon node takes no p2p bid whose value plus execution payment is + /// below it, however the boost factor weighs it. + /// + /// Sent as `produceBlockV4`'s `min_bid`, so gloas only. A bid floor + /// matters most with `--prefer-builder-proposals`, which otherwise takes + /// a 1-gwei bid over the local payload. `0` sets no floor. + #[arg(long, default_value_t = 0, requires = "builder_proposals")] + pub(crate) builder_min_bid: u64, + /// Register every loaded key that has no slashing-protection record, /// instead of refusing to start. /// @@ -191,6 +201,7 @@ impl ValidatorOptions { } else { self.builder_boost_factor }, + min_bid: self.builder_min_bid, }) } @@ -255,6 +266,7 @@ mod tests { builder_proposals: false, builder_boost_factor: 100, prefer_builder_proposals: false, + builder_min_bid: 0, init_slashing_protection: false, } } @@ -306,7 +318,10 @@ mod tests { let options = parse(&["--builder-proposals"]).expect("parses"); assert_eq!( options.builder(), - Some(BuilderProposals { boost_factor: 100 }) + Some(BuilderProposals { + boost_factor: 100, + min_bid: 0 + }) ); } @@ -316,7 +331,10 @@ mod tests { parse(&["--builder-proposals", "--builder-boost-factor", "250"]).expect("parses"); assert_eq!( options.builder(), - Some(BuilderProposals { boost_factor: 250 }) + Some(BuilderProposals { + boost_factor: 250, + min_bid: 0 + }) ); } @@ -327,7 +345,8 @@ mod tests { assert_eq!( options.builder(), Some(BuilderProposals { - boost_factor: u64::MAX + boost_factor: u64::MAX, + min_bid: 0 }) ); } @@ -338,6 +357,36 @@ mod tests { .expect_err("a factor with builder proposals off is a mistake"); parse(&["--prefer-builder-proposals"]) .expect_err("a preference with builder proposals off is a mistake"); + parse(&["--builder-min-bid", "1000000"]) + .expect_err("a bid floor with builder proposals off is a mistake"); + } + + /// The floor travels with either way of weighing bids, in gwei as given. + #[test] + fn a_min_bid_is_used_beside_the_factor_or_the_preference() { + let options = + parse(&["--builder-proposals", "--builder-min-bid", "1000000"]).expect("parses"); + assert_eq!( + options.builder(), + Some(BuilderProposals { + boost_factor: 100, + min_bid: 1_000_000 + }) + ); + let options = parse(&[ + "--builder-proposals", + "--prefer-builder-proposals", + "--builder-min-bid", + "5", + ]) + .expect("parses"); + assert_eq!( + options.builder(), + Some(BuilderProposals { + boost_factor: u64::MAX, + min_bid: 5 + }) + ); } #[test] diff --git a/crates/net/rpc/src/beacon/validator_client_tests.rs b/crates/net/rpc/src/beacon/validator_client_tests.rs index 88c8a7951..a85782812 100644 --- a/crates/net/rpc/src/beacon/validator_client_tests.rs +++ b/crates/net/rpc/src/beacon/validator_client_tests.rs @@ -431,6 +431,7 @@ async fn block_production_without_an_execution_client_is_retryable() { randao_reveal: Default::default(), graffiti: Default::default(), builder_boost_factor: None, + builder_min_bid: 0, }; let err = client.produce_block(&request).await.unwrap_err(); assert!(matches!(err, Error::BeaconNodeSyncing), "got {err:?}"); @@ -672,6 +673,7 @@ async fn produce_with_pool( randao_reveal, graffiti: ethlambda_types::primitives::H256(graffiti), builder_boost_factor: None, + builder_min_bid: 0, }; let produced = client.produce_block(&request).await.unwrap(); assert_eq!(produced.slot(), slot); @@ -1134,6 +1136,7 @@ async fn the_validator_client_can_serve_on_the_sync_committee() { randao_reveal, graffiti: Default::default(), builder_boost_factor: None, + builder_min_bid: 0, }; let produced = client.produce_block(&request).await.unwrap(); let block_domain = get_domain(&advanced, DOMAIN_BEACON_PROPOSER, Some(epoch)); @@ -1852,6 +1855,7 @@ mod builder_flow { randao_reveal, graffiti: ethlambda_types::primitives::H256([0; 32]), builder_boost_factor, + builder_min_bid: 0, }; self.client.produce_block(&request).await.unwrap() } diff --git a/crates/validator/src/beacon_node/fallback.rs b/crates/validator/src/beacon_node/fallback.rs index 24b2d813f..d4a68037c 100644 --- a/crates/validator/src/beacon_node/fallback.rs +++ b/crates/validator/src/beacon_node/fallback.rs @@ -836,6 +836,7 @@ mod tests { randao_reveal: BlsSignature([0; 96]), graffiti: Bytes32::default(), builder_boost_factor: None, + builder_min_bid: 0, } } diff --git a/crates/validator/src/beacon_node/http.rs b/crates/validator/src/beacon_node/http.rs index b5147acf1..f896e68c5 100644 --- a/crates/validator/src/beacon_node/http.rs +++ b/crates/validator/src/beacon_node/http.rs @@ -134,13 +134,15 @@ struct SszResponse { /// The body `produceBlockV4` requires: the node decodes it, as the /// specification demands, even when it builds locally. /// -/// `min_bid` stays `0` and `builders` stays empty, since the builder API -/// (relay or `builder_pubkeys` bids) is a later phase. The factor is the -/// configured `builder_boost_factor` with `--builder-proposals`, so a pooled -/// p2p bid can win on value, and `0` without it: self-build only, as before. -fn builder_config(boost_factor: Option) -> String { +/// `builders` stays empty, since the builder API (relay or `builder_pubkeys` +/// bids) is a later phase. The top-level `min_bid` and factor govern the pooled +/// p2p bids. The factor is the configured `builder_boost_factor` with +/// `--builder-proposals`, so such a bid can win on value, and `0` without it: +/// self-build only, as before. `min_bid` is `--builder-min-bid`, in gwei, the +/// floor below which no p2p bid is taken, `0` without builder proposals. +fn builder_config(boost_factor: Option, min_bid: u64) -> String { let factor = boost_factor.unwrap_or(0); - format!(r#"{{"min_bid":"0","builder_boost_factor":"{factor}","builders":[]}}"#) + format!(r#"{{"min_bid":"{min_bid}","builder_boost_factor":"{factor}","builders":[]}}"#) } /// A [`BeaconNodeApi`] backed by one beacon node's standard REST Beacon API. @@ -454,7 +456,7 @@ impl HttpBeaconNode { let response = self .post_json_for_ssz( &path, - builder_config(request.builder_boost_factor), + builder_config(request.builder_boost_factor, request.builder_min_bid), request.fork.as_str(), ) .await?; @@ -1409,23 +1411,31 @@ mod tests { /// The body `produceBlockV4` requires: no minimum bid and no builders, /// the boost factor being the configured one or `0`. #[test] - fn the_builder_config_carries_the_factor_or_zero() { + fn the_builder_config_carries_the_factor_or_zero_and_the_min_bid() { for (factor, expected) in [(None, "0"), (Some(250), "250"), (Some(0), "0")] { let value: serde_json::Value = - serde_json::from_str(&builder_config(factor)).expect("valid json"); + serde_json::from_str(&builder_config(factor, 0)).expect("valid json"); assert_eq!(value["min_bid"], "0"); assert_eq!(value["builder_boost_factor"], expected, "{factor:?}"); assert_eq!(value["builders"], serde_json::json!([])); } + let value: serde_json::Value = + serde_json::from_str(&builder_config(Some(100), u64::MAX)).expect("valid json"); + assert_eq!(value["min_bid"], u64::MAX.to_string(), "quoted, as a Gwei"); } /// The body reaches the wire: a gloas production request posts it. #[tokio::test] - async fn a_gloas_production_request_posts_the_factor_in_its_body() { - for (factor, expected) in [(None, "0"), (Some(250), "250")] { + async fn a_gloas_production_request_posts_the_factor_and_min_bid_in_its_body() { + let cases = [ + (None, 0, "0", "0"), + (Some(250), 1_000_000, "250", "1000000"), + ]; + for (factor, min_bid, expected_factor, expected_min_bid) in cases { let served = serve(500, vec![], vec![]).await; let request = BlockRequest { fork: ForkName::Gloas, + builder_min_bid: min_bid, ..block_request(factor) }; let _ = served.node.produce_block(&request).await; @@ -1433,7 +1443,8 @@ mod tests { assert_eq!(seen.len(), 1); assert!(seen[0].0.contains("/eth/v4/validator/blocks/5")); let body: serde_json::Value = serde_json::from_slice(&seen[0].1).expect("json body"); - assert_eq!(body["builder_boost_factor"], expected); + assert_eq!(body["builder_boost_factor"], expected_factor); + assert_eq!(body["min_bid"], expected_min_bid); } } @@ -1500,6 +1511,7 @@ mod tests { randao_reveal: Default::default(), graffiti: Default::default(), builder_boost_factor: factor, + builder_min_bid: 0, } } diff --git a/crates/validator/src/beacon_node/mod.rs b/crates/validator/src/beacon_node/mod.rs index 9e864fbe0..2bbccba47 100644 --- a/crates/validator/src/beacon_node/mod.rs +++ b/crates/validator/src/beacon_node/mod.rs @@ -149,6 +149,10 @@ pub struct BlockRequest { /// `produceBlockV4`'s body instead of the query, so a pooled p2p bid can /// win on value; a gloas answer is never blinded. pub builder_boost_factor: Option, + /// The smallest p2p bid, in gwei, the node may take for a gloas block + /// (`produceBlockV4`'s `min_bid`): `--builder-min-bid`, `0` when builder + /// proposals are off. Not sent before gloas, where no endpoint takes it. + pub builder_min_bid: u64, } /// What became of a published block. diff --git a/crates/validator/src/lib.rs b/crates/validator/src/lib.rs index 98f084021..226e7f7aa 100644 --- a/crates/validator/src/lib.rs +++ b/crates/validator/src/lib.rs @@ -118,6 +118,9 @@ pub struct BuilderProposals { /// bid and the local payload as they are; `u64::MAX` takes the builder /// whenever there is a bid. pub boost_factor: u64, + /// The smallest p2p bid, in gwei, a gloas block may be built on + /// (`produceBlockV4`'s `min_bid`). `0` takes any bid that wins on value. + pub min_bid: u64, } /// Run the validator client until the process is stopped. @@ -263,10 +266,12 @@ pub async fn run(config: ValidatorConfig) -> Result<()> { config.builder.map(|builder| builder.boost_factor), ) .with_clock(clock.clone()) + .with_builder_min_bid(config.builder.map_or(0, |builder| builder.min_bid)) .with_preferences(preferences.clone()); let registration = config.builder.map(|builder| { info!( boost_factor = builder.boost_factor, + min_bid_gwei = builder.min_bid, "Builder proposals on: registering validators and accepting blinded blocks" ); Arc::new(RegistrationService::new( diff --git a/crates/validator/src/proposal.rs b/crates/validator/src/proposal.rs index 01ac62888..79e72ff12 100644 --- a/crates/validator/src/proposal.rs +++ b/crates/validator/src/proposal.rs @@ -114,6 +114,9 @@ pub struct ProposalService { /// The `builder_boost_factor` sent with each block request, or `None` when /// builder proposals are off, in which case a blinded answer is refused. builder_boost_factor: Option, + /// The `min_bid` sent with each gloas block request, in gwei: the floor + /// below which the node takes no p2p bid. `0` unless set. + builder_min_bid: u64, /// The slot of the latest blinded block this service signed, set before /// its publish starts. A `std::sync::Mutex` because the critical section /// is a single store with no await inside it. @@ -141,12 +144,20 @@ impl ProposalService { settings, protection, builder_boost_factor, + builder_min_bid: 0, last_blinded_slot: std::sync::Mutex::new(None), clock: None, preferences: None, } } + /// Send `min_bid` (gwei) with every gloas block request, so the node + /// takes no p2p bid below it. + pub fn with_builder_min_bid(mut self, min_bid: u64) -> Self { + self.builder_min_bid = min_bid; + self + } + /// Give the service the proposer preferences this client published, so /// the fee recipient check accepts the address a gloas block's node was /// given in them. @@ -300,6 +311,7 @@ impl ProposalService { randao_reveal, graffiti, builder_boost_factor: self.builder_boost_factor, + builder_min_bid: self.builder_min_bid, }; let mut produced = self.beacon_node.produce_block(&request).await?; @@ -1086,6 +1098,7 @@ mod tests { randao_reveal: Default::default(), graffiti: Default::default(), builder_boost_factor: None, + builder_min_bid: 0, }; let full = full.produce_block(&request).await.expect("produces"); assert!(ProposalService::::compares_fee_recipient( @@ -1112,6 +1125,23 @@ mod tests { assert_eq!(pays(stranger, None, &[]), None); } + /// The configured floor goes with the block request, beside the factor. + #[tokio::test] + async fn the_min_bid_goes_with_every_block_request() { + let (store, pubkey) = store(); + let node = Arc::new(MockBeaconNode::new().with_block(slot(), 7)); + let service = builder_service(node.clone(), None).with_builder_min_bid(1_000_000); + + service + .propose(slot(), &duty(&pubkey, 7, slot()), &store) + .await + .expect("proposes"); + + let requests = node.block_requests(); + assert_eq!(requests[0].builder_boost_factor, Some(100)); + assert_eq!(requests[0].builder_min_bid, 1_000_000); + } + /// A node that does not confirm the publish may still have handed the block /// to its relay, so it is not a missed proposal, and the slot is spent: a /// second block would be an equivocation. diff --git a/docs/cli.md b/docs/cli.md index a71a699ba..2aca94655 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -304,6 +304,7 @@ implementation's, so none of the [common flags](#common-flags) apply to it. | `--builder-proposals` | off | Propose builder blocks: register each validator with the beacon node's builder (an ethlambda node: `--builder`) and accept a blinded block when the builder's bid wins. Registrations and blinded blocks are fulu and earlier only. From gloas the builder market works through proposer preferences, which are published with or without this flag; there it only sets the boost factor that weighs p2p bids in `produceBlockV4` | | `--builder-boost-factor` | `100` | How a builder's bid is weighed against the local payload: the local payload wins iff `local >= factor * bid / 100`. `0` prefers the local payload unless it cannot be built. From gloas it is also sent in `produceBlockV4`'s body, so a pooled p2p bid can win on value; without `--builder-proposals` the body carries `0`. Requires `--builder-proposals`; conflicts with `--prefer-builder-proposals` | | `--prefer-builder-proposals` | off | Take the builder's block whenever there is a valid bid, unless the execution client asks to override the builder (sends `builder_boost_factor = u64::MAX`). Requires `--builder-proposals`; conflicts with `--builder-boost-factor` | +| `--builder-min-bid` | `0` | The smallest builder bid, in gwei, a gloas block may be built on: sent as `produceBlockV4`'s `min_bid`, so the node takes no p2p bid whose value plus execution payment is below it, however the boost factor weighs it. Gloas only. Worth setting with `--prefer-builder-proposals`, which otherwise takes a 1-gwei bid over the local payload. `0` sets no floor. Requires `--builder-proposals` | | `--init-slashing-protection` | off | Register every loaded key with no slashing-protection record as never having signed, instead of refusing to start. Dangerous: only for keys that have signed nothing anywhere for at least two epochs, and even then it rules out a double vote only, not a surround of an older vote; a key with a history should bring it instead. Never lowers an existing record; remove it once the keys are registered, and startup warns while it is set for nothing | ### What it does today diff --git a/docs/spec_deviations.md b/docs/spec_deviations.md index 28ac90cdd..a24c9b21b 100644 --- a/docs/spec_deviations.md +++ b/docs/spec_deviations.md @@ -797,7 +797,8 @@ signature, whichever way the node answered. Three departures: - **Registrations stop at gloas.** The client sends no `register_validator` from the gloas fork epoch on: the builder market works through proposer preferences there (see the next section), and `produceBlockV4` sends the - configured boost factor with no minimum bid and no builders. + configured boost factor and `--builder-min-bid` (default `0`), and no + builders. Validators with no fee recipient are not registered either (the field is required), and only validators the beacon node resolved are signed for.