diff --git a/CLAUDE.md b/CLAUDE.md index aa67c4bca..dec9283c7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -862,7 +862,9 @@ transitions are in `ethlambda-types`, per the section above. Nothing above and a block at slot N packs only `(N-1, parent_root)`, replaced by the empty aggregate when `verified_sync_aggregate` fails (see `docs/spec_deviations.md`). `SlotClock` picks the - offsets by each slot's own fork, and `/eth/v1/config/spec` supplies them. It + 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 `(validator, slot)` dedup only, and envelopes are unguarded. `get_head_node` is one bottom-up walk for every fork, with diff --git a/README.md b/README.md index 29d348aa5..95d606278 100644 --- a/README.md +++ b/README.md @@ -356,6 +356,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 | +| `--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 | Metrics are served on `--metrics-port` (default `5064`). See diff --git a/bin/ethlambda/src/validator.rs b/bin/ethlambda/src/validator.rs index f97f82782..51b27a15a 100644 --- a/bin/ethlambda/src/validator.rs +++ b/bin/ethlambda/src/validator.rs @@ -6,8 +6,8 @@ use std::path::PathBuf; use ethlambda_types::beacon::primitives::{Bytes32, ExecutionAddress, H160}; -use ethlambda_validator::ValidatorConfig; use ethlambda_validator::proposer_settings::{GRAFFITI_BYTES, graffiti_from_text}; +use ethlambda_validator::{BuilderProposals, ValidatorConfig}; /// An execution address is exactly this wide. const ADDRESS_BYTES: usize = 20; @@ -78,6 +78,31 @@ pub(crate) struct ValidatorOptions { /// beyond localhost must front it with TLS. #[arg(long, default_value = "false")] pub(crate) enable_keymanager: bool, + + /// 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. + #[arg(long, default_value = "false")] + pub(crate) builder_proposals: bool, + + /// How a builder's bid is weighed against the local payload: the local + /// payload wins iff `local >= factor * bid / 100`. 100 compares them as + /// they are; 0 prefers the local payload unless it cannot be built. + #[arg( + long, + default_value = "100", + requires = "builder_proposals", + conflicts_with = "prefer_builder_proposals" + )] + pub(crate) builder_boost_factor: u64, + + /// 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`). + #[arg(long, default_value = "false", requires = "builder_proposals")] + pub(crate) prefer_builder_proposals: bool, } impl ValidatorOptions { @@ -121,6 +146,21 @@ impl ValidatorOptions { Ok(Some(H160(bytes))) } + /// The builder configuration these flags describe, or `None` when builder + /// proposals are off. + /// + /// `--prefer-builder-proposals` wins over the factor by construction: + /// clap refuses the two together, so at most one is ever set. + pub(crate) fn builder(&self) -> Option { + self.builder_proposals.then_some(BuilderProposals { + boost_factor: if self.prefer_builder_proposals { + u64::MAX + } else { + self.builder_boost_factor + }, + }) + } + /// Reject a port clash before anything binds, the way the node's own /// options do. pub(crate) fn validate_ports(&self) -> eyre::Result<()> { @@ -144,6 +184,7 @@ pub(crate) async fn run(options: ValidatorOptions) -> eyre::Result<()> { options.validate_ports()?; let graffiti = options.graffiti()?; let suggested_fee_recipient = options.suggested_fee_recipient()?; + let builder = options.builder(); ethlambda_validator::run(ValidatorConfig { beacon_nodes: options.beacon_nodes, graffiti, @@ -154,6 +195,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)), + builder, }) .await .map_err(Into::into) @@ -174,7 +216,87 @@ mod tests { graffiti: graffiti.to_string(), suggested_fee_recipient: None, enable_keymanager: false, + builder_proposals: false, + builder_boost_factor: 100, + prefer_builder_proposals: false, + } + } + + /// Parse a command line the way the binary does, so `requires` and + /// `conflicts_with` are exercised rather than assumed. + fn parse(extra: &[&str]) -> Result { + use clap::Parser as _; + + #[derive(clap::Parser)] + struct Cli { + #[command(flatten)] + options: ValidatorOptions, } + let base = [ + "ethlambda", + "--beacon-nodes", + "http://localhost:5052", + "--validators-dir", + "/tmp/v", + "--secrets-dir", + "/tmp/s", + ]; + Cli::try_parse_from(base.iter().chain(extra)).map(|cli| cli.options) + } + + #[test] + fn builder_proposals_are_off_by_default() { + assert_eq!(parse(&[]).expect("parses").builder(), None); + } + + #[test] + fn builder_proposals_default_to_a_factor_of_100() { + let options = parse(&["--builder-proposals"]).expect("parses"); + assert_eq!( + options.builder(), + Some(BuilderProposals { boost_factor: 100 }) + ); + } + + #[test] + fn an_explicit_factor_is_used() { + let options = + parse(&["--builder-proposals", "--builder-boost-factor", "250"]).expect("parses"); + assert_eq!( + options.builder(), + Some(BuilderProposals { boost_factor: 250 }) + ); + } + + #[test] + fn preferring_the_builder_sends_the_maximum_factor() { + let options = + parse(&["--builder-proposals", "--prefer-builder-proposals"]).expect("parses"); + assert_eq!( + options.builder(), + Some(BuilderProposals { + boost_factor: u64::MAX + }) + ); + } + + #[test] + fn the_factor_and_the_preference_need_builder_proposals() { + parse(&["--builder-boost-factor", "250"]) + .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"); + } + + #[test] + fn the_factor_and_the_preference_conflict() { + parse(&[ + "--builder-proposals", + "--builder-boost-factor", + "250", + "--prefer-builder-proposals", + ]) + .expect_err("the two say different things"); } #[test] diff --git a/crates/net/rpc/src/beacon/proposal.rs b/crates/net/rpc/src/beacon/proposal.rs index c5d0e66f8..30d4573f7 100644 --- a/crates/net/rpc/src/beacon/proposal.rs +++ b/crates/net/rpc/src/beacon/proposal.rs @@ -1083,6 +1083,7 @@ pub(crate) fn decimal(value: ðlambda_types::beacon::primitives::Uint256) -> S #[cfg(test)] mod tests { + use super::test_support::*; use super::*; use crate::test_utils::{RecordingNetwork, beacon_store_with_config}; use axum::{body::Body, http::Request}; @@ -1109,28 +1110,6 @@ mod tests { (status, serde_json::from_slice(&body).unwrap_or_default()) } - /// A fulu head with its proposer lookahead and sync committee filled in, - /// which a block built on it needs, under a schedule that puts every slot - /// at fulu. - fn fulu_store() -> (Store, Slot) { - use ethlambda_state_transition::beacon::helpers::{ - altair::get_next_sync_committee, fulu::initialize_proposer_lookahead, - }; - let mut state = with_signing_validators_at(ForkName::Fulu, 64); - let lookahead = initialize_proposer_lookahead(&state).unwrap(); - let sync_committee = get_next_sync_committee(&state).unwrap(); - let BeaconState::Fulu(inner) = &mut state else { - unreachable!("built as fulu") - }; - inner.proposer_lookahead = lookahead.try_into().unwrap(); - inner.current_sync_committee = sync_committee.clone(); - inner.next_sync_committee = sync_committee; - let head_slot = state.slot(); - let (store, _root) = - beacon_store_with_config(state, Config::mainnet().with_fork_epoch(ForkName::Fulu, 0)); - (store, head_slot) - } - fn produce_app(store: Store, engine: Option) -> Router { routes() .with_state(store) @@ -1414,116 +1393,14 @@ mod tests { assert!(rejected.blocks.lock().unwrap().is_empty()); } - use crate::beacon::builder_client::mock_relay::MockRelay; - use crate::beacon::builder_client::{BuilderClient, VersionedBid}; - use crate::beacon::builder_flow::breaker::FallbackThresholds; + use crate::beacon::builder_client::BuilderClient; use ethlambda_blockchain::SyncStatusController; - use ethlambda_types::beacon::builder::{BuilderBid, ValidatorRegistrationV1}; - use ethlambda_types::beacon::containers::capella; - use ethlambda_types::beacon::containers::deneb::{ExecutionPayloadHeader, KzgCommitments}; - use ethlambda_types::beacon::containers::electra::{self, ExecutionRequests}; + use ethlambda_types::beacon::builder::ValidatorRegistrationV1; + use ethlambda_types::beacon::containers::electra; use libssz::SszDecode as _; use std::time::Duration; - const ETH: u64 = 1_000_000_000_000_000_000; - /// The registered gas limit the test proposer asks for. - const TARGET_GAS_LIMIT: u64 = 60_000_000; - - /// Thresholds that never trip, so a test state's history does not decide - /// whether the builder is asked. - const NEVER_TRIP: FallbackThresholds = FallbackThresholds { - skips: u64::MAX, - skips_per_epoch: u64::MAX, - epochs_since_finalization: u64::MAX, - }; - - /// A builder world for the next slot on `fulu_store()`: a mock relay, a - /// `Builder` pointing at it with the proposer registered, and the - /// advanced proposal state. - struct BuilderWorld { - store: Store, - slot: Slot, - state: BeaconState, - relay: MockRelay, - builder: Arc, - } - - async fn builder_world(registered: bool, sync: SyncStatus) -> BuilderWorld { - let (store, head_slot) = fulu_store(); - let slot = head_slot + 1; - let config = store.config(); - let (_, head_state) = super::super::validator::head(&store).unwrap(); - let state = advance_to_slot(&head_state, slot, &config).unwrap(); - let proposer = get_beacon_proposer_index(&state).unwrap(); - let pubkey = state.validator(proposer).unwrap().pubkey; - let relay = MockRelay::start(config.genesis_fork_version).await; - let client = - BuilderClient::new(relay.url.clone(), Duration::from_secs(1), "ethlambda/test") - .unwrap(); - let builder = Arc::new(Builder::new( - client, - NEVER_TRIP, - SyncStatusController::new(sync), - )); - if registered { - let registration = ValidatorRegistrationV1 { - fee_recipient: ExecutionAddress::repeat_byte(0x11), - gas_limit: TARGET_GAS_LIMIT, - timestamp: 1, - pubkey, - }; - builder - .registrations - .record([®istration], compute_epoch_at_slot(slot)); - } - BuilderWorld { - store, - slot, - state, - relay, - builder, - } - } - impl BuilderWorld { - /// A bid of `value` wei that passes every check for this slot, with - /// `edit` applied before the relay signs it. - fn offer(&self, value: u64, edit: impl FnOnce(&mut BuilderBid)) { - let config = self.store.config(); - let inputs = payload_inputs(&self.state, &config).unwrap(); - let BeaconState::Fulu(fulu) = &self.state else { - unreachable!("built as fulu") - }; - let parent_gas_limit = fulu.latest_execution_payload_header.gas_limit; - let step = (parent_gas_limit / 1024).saturating_sub(1); - let withdrawals: capella::Withdrawals = inputs.withdrawals.clone().try_into().unwrap(); - let mut header = - ExecutionPayloadHeader::from(&electra::BeaconBlockBody::empty().execution_payload); - header.parent_hash = inputs.parent_hash; - header.prev_randao = inputs.prev_randao; - header.timestamp = inputs.timestamp; - header.block_number = fulu.latest_execution_payload_header.block_number + 1; - header.withdrawals_root = withdrawals.hash_tree_root(); - header.gas_limit = TARGET_GAS_LIMIT.clamp( - parent_gas_limit - step, - parent_gas_limit.saturating_add(step), - ); - header.block_hash = ExecutionBlockHash::repeat_byte(0xbb); - let mut message = BuilderBid { - header, - blob_kzg_commitments: KzgCommitments::default(), - execution_requests: ExecutionRequests::default(), - value: Uint256::from(value), - pubkey: self.relay.pubkey(), - }; - edit(&mut message); - let bid = self.relay.sign_bid(message); - self.relay.offer(VersionedBid { - fork: ForkName::Fulu, - bid, - }); - } - fn app(&self, engine: Option) -> Router { produce_app(self.store.clone(), engine).layer(Extension(self.builder.clone())) } @@ -2023,3 +1900,145 @@ mod tests { assert_eq!(status, StatusCode::PAYLOAD_TOO_LARGE); } } + +/// The builder world the proposal tests build on: a fulu head, a mock relay and +/// a `Builder` pointing at it. Shared with the validator client's end-to-end +/// tests, which drive the same routes through a real socket. +#[cfg(test)] +pub(crate) mod test_support { + use super::*; + use crate::beacon::builder_client::BuilderClient; + use crate::beacon::builder_client::mock_relay::MockRelay; + use crate::beacon::builder_flow::breaker::FallbackThresholds; + use crate::test_utils::beacon_store_with_config; + use ethlambda_blockchain::SyncStatusController; + use ethlambda_state_transition::beacon::helpers::test_state::with_signing_validators_at; + use ethlambda_types::beacon::builder::ValidatorRegistrationV1; + use ethlambda_types::beacon::config::Config; + use ethlambda_types::beacon::containers::capella; + use ethlambda_types::beacon::containers::deneb::{ExecutionPayloadHeader, KzgCommitments}; + use ethlambda_types::beacon::containers::electra; + use std::time::Duration; + + /// A fulu head with its proposer lookahead and sync committee filled in, + /// which a block built on it needs, under a schedule that puts every slot + /// at fulu. + pub(crate) fn fulu_store() -> (Store, Slot) { + use ethlambda_state_transition::beacon::helpers::{ + altair::get_next_sync_committee, fulu::initialize_proposer_lookahead, + }; + let mut state = with_signing_validators_at(ForkName::Fulu, 64); + let lookahead = initialize_proposer_lookahead(&state).unwrap(); + let sync_committee = get_next_sync_committee(&state).unwrap(); + let BeaconState::Fulu(inner) = &mut state else { + unreachable!("built as fulu") + }; + inner.proposer_lookahead = lookahead.try_into().unwrap(); + inner.current_sync_committee = sync_committee.clone(); + inner.next_sync_committee = sync_committee; + let head_slot = state.slot(); + let (store, _root) = + beacon_store_with_config(state, Config::mainnet().with_fork_epoch(ForkName::Fulu, 0)); + (store, head_slot) + } + + pub(crate) const ETH: u64 = 1_000_000_000_000_000_000; + /// The registered gas limit the test proposer asks for. + pub(crate) const TARGET_GAS_LIMIT: u64 = 60_000_000; + + /// Thresholds that never trip, so a test state's history does not decide + /// whether the builder is asked. + pub(crate) const NEVER_TRIP: FallbackThresholds = FallbackThresholds { + skips: u64::MAX, + skips_per_epoch: u64::MAX, + epochs_since_finalization: u64::MAX, + }; + + /// A builder world for the next slot on `fulu_store()`: a mock relay, a + /// `Builder` pointing at it with the proposer registered, and the + /// advanced proposal state. + pub(crate) struct BuilderWorld { + pub(crate) store: Store, + pub(crate) slot: Slot, + pub(crate) state: BeaconState, + pub(crate) relay: MockRelay, + pub(crate) builder: Arc, + } + + pub(crate) async fn builder_world(registered: bool, sync: SyncStatus) -> BuilderWorld { + let (store, head_slot) = fulu_store(); + let slot = head_slot + 1; + let config = store.config(); + let (_, head_state) = super::super::validator::head(&store).unwrap(); + let state = advance_to_slot(&head_state, slot, &config).unwrap(); + let proposer = get_beacon_proposer_index(&state).unwrap(); + let pubkey = state.validator(proposer).unwrap().pubkey; + let relay = MockRelay::start(config.genesis_fork_version).await; + let client = + BuilderClient::new(relay.url.clone(), Duration::from_secs(1), "ethlambda/test") + .unwrap(); + let builder = Arc::new(Builder::new( + client, + NEVER_TRIP, + SyncStatusController::new(sync), + )); + if registered { + let registration = ValidatorRegistrationV1 { + fee_recipient: ExecutionAddress::repeat_byte(0x11), + gas_limit: TARGET_GAS_LIMIT, + timestamp: 1, + pubkey, + }; + builder + .registrations + .record([®istration], compute_epoch_at_slot(slot)); + } + BuilderWorld { + store, + slot, + state, + relay, + builder, + } + } + + impl BuilderWorld { + /// A bid of `value` wei that passes every check for this slot, with + /// `edit` applied before the relay signs it. + pub(crate) fn offer(&self, value: u64, edit: impl FnOnce(&mut BuilderBid)) { + let config = self.store.config(); + let inputs = payload_inputs(&self.state, &config).unwrap(); + let BeaconState::Fulu(fulu) = &self.state else { + unreachable!("built as fulu") + }; + let parent_gas_limit = fulu.latest_execution_payload_header.gas_limit; + let step = (parent_gas_limit / 1024).saturating_sub(1); + let withdrawals: capella::Withdrawals = inputs.withdrawals.clone().try_into().unwrap(); + let mut header = + ExecutionPayloadHeader::from(&electra::BeaconBlockBody::empty().execution_payload); + header.parent_hash = inputs.parent_hash; + header.prev_randao = inputs.prev_randao; + header.timestamp = inputs.timestamp; + header.block_number = fulu.latest_execution_payload_header.block_number + 1; + header.withdrawals_root = withdrawals.hash_tree_root(); + header.gas_limit = TARGET_GAS_LIMIT.clamp( + parent_gas_limit - step, + parent_gas_limit.saturating_add(step), + ); + header.block_hash = ExecutionBlockHash::repeat_byte(0xbb); + let mut message = BuilderBid { + header, + blob_kzg_commitments: KzgCommitments::default(), + execution_requests: ExecutionRequests::default(), + value: Uint256::from(value), + pubkey: self.relay.pubkey(), + }; + edit(&mut message); + let bid = self.relay.sign_bid(message); + self.relay.offer(VersionedBid { + fork: ForkName::Fulu, + bid, + }); + } + } +} diff --git a/crates/net/rpc/src/beacon/validator_client_tests.rs b/crates/net/rpc/src/beacon/validator_client_tests.rs index 654fb6d5d..018471e06 100644 --- a/crates/net/rpc/src/beacon/validator_client_tests.rs +++ b/crates/net/rpc/src/beacon/validator_client_tests.rs @@ -131,16 +131,20 @@ async fn serve_at( .unwrap(); let parent_root = advanced.latest_block_header().hash_tree_root(); store.insert_state(parent_root, state.clone()).unwrap(); - let (client, network, _pool, address) = spawn_server(store.clone(), engine).await; + let (client, network, _pool, address) = spawn_server(store.clone(), engine, None).await; (client, state, network, address, store) } /// Serve `store` through the router the real server builds, over a socket on an /// ephemeral port, and connect this crate's client to it. Also hands back the /// payload attestation pool the router fills and the socket it listens on. +/// +/// With a `builder`, it is layered as `lib.rs` does when `--builder` is set, +/// so the builder flow's endpoints see it. async fn spawn_server( mut store: Store, engine: Option, + builder: Option>, ) -> ( HttpBeaconNode, Arc, @@ -171,6 +175,10 @@ async fn spawn_server( .layer(Extension(crate::beacon::graffiti::OwnVersion(Arc::new( own_version(), )))); + let router = match builder { + Some(builder) => router.layer(Extension(builder)), + None => router, + }; let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); let address = listener.local_addr().unwrap(); @@ -393,6 +401,7 @@ async fn block_production_without_an_execution_client_is_retryable() { proposer_index: 0, randao_reveal: Default::default(), graffiti: Default::default(), + builder_boost_factor: None, }; let err = client.produce_block(&request).await.unwrap_err(); assert!(matches!(err, Error::BeaconNodeSyncing), "got {err:?}"); @@ -610,6 +619,7 @@ async fn produce_with_pool( proposer_index: proposer, randao_reveal, graffiti: ethlambda_types::primitives::H256(graffiti), + builder_boost_factor: None, }; let produced = client.produce_block(&request).await.unwrap(); assert_eq!(produced.slot(), slot); @@ -644,6 +654,8 @@ fn fulu_block( match &produced.contents { Contents::WithCellProofs { block, .. } | Contents::WithBlobProofs { block, .. } => block, Contents::Gloas { .. } => unreachable!("a fulu proposal"), + // These tests ask with no `builder_boost_factor`, so no blinded answer. + Contents::Blinded { .. } => unreachable!("builder proposals are off"), } } @@ -870,7 +882,7 @@ async fn serve_fulu_pinned( head_root, slot, ); - let (client, network, _, _) = spawn_server(store, engine).await; + let (client, network, _, _) = spawn_server(store, engine, None).await; let genesis = client.genesis().await.expect("genesis"); let spec = client .spec() @@ -1060,6 +1072,7 @@ async fn the_validator_client_can_serve_on_the_sync_committee() { proposer_index: proposer, randao_reveal, graffiti: Default::default(), + builder_boost_factor: None, }; let produced = client.produce_block(&request).await.unwrap(); let block_domain = get_domain(&advanced, DOMAIN_BEACON_PROPOSER, Some(epoch)); @@ -1170,7 +1183,8 @@ mod gloas { let head_root = header.hash_tree_root(); let store = beacon_store_with_head_block(state.clone(), config(), block, head_root, clock_slot); - let (client, network, payload_pool, _address) = spawn_server(store.clone(), engine).await; + let (client, network, payload_pool, _address) = + spawn_server(store.clone(), engine, None).await; let genesis = client.genesis().await.expect("genesis"); let spec = client .spec() @@ -1499,6 +1513,7 @@ mod gloas { None, ), ), + None, ); let duty = ProposerDutyDto { pubkey: encode_hex(&pubkey.0), @@ -1617,6 +1632,7 @@ mod gloas { None, ), ), + None, ); let duty = ProposerDutyDto { pubkey: encode_hex(&pubkey.0), @@ -1636,3 +1652,206 @@ mod gloas { assert_full_sync_aggregate(&advanced, &signed.message.body.sync_aggregate); } } + +/// The builder flow through this node: the validator client's own +/// registration, `produceBlockV3` with a boost factor, and blinded publish, +/// against this node's routes with a `Builder` layered over a mock relay. +mod builder_flow { + use super::*; + use crate::beacon::proposal::test_support::{ + BuilderWorld, ETH, TARGET_GAS_LIMIT, builder_world, + }; + use ethlambda_state_transition::beacon::helpers::accessors::get_beacon_proposer_index; + use ethlambda_types::beacon::builder::{ + SignedBlindedBeaconBlock, SignedValidatorRegistrationV1, ValidatorRegistrationV1, + }; + use ethlambda_types::beacon::primitives::ExecutionAddress; + use ethlambda_validator::beacon_node::Published; + use libssz::SszDecode as _; + + /// What a test drives: this node served over a socket, the proposer's key + /// and the client's signing context. + struct Harness { + world: BuilderWorld, + client: HttpBeaconNode, + proposer: u64, + keys: RwLock, + context: SigningContext, + } + + /// `world`'s store served through the real beacon router, with its + /// `Builder` layered, behind a socket. + async fn harness(world: BuilderWorld, engine: ethlambda_engine::EngineClient) -> Harness { + // A block built on this head names the advanced header's root as its + // parent, which the fixture's placeholder anchor block does not have: + // keep the head state under that root too, as `serve_at` does, so the + // publish finds its parent. + let mut store = world.store.clone(); + let (_, head_state) = crate::beacon::validator::head(&store).unwrap(); + let parent_root = world.state.latest_block_header().hash_tree_root(); + store + .insert_state(parent_root, (*head_state).clone()) + .unwrap(); + + let (client, _network, _pool, _address) = + spawn_server(store, Some(engine), Some(world.builder.clone())).await; + + let proposer = get_beacon_proposer_index(&world.state).unwrap(); + let keys = keys_for(&world.state, &[proposer]); + let context = SigningContext { + config: (*world.store.config()).clone(), + genesis_validators_root: world.state.genesis_validators_root(), + }; + Harness { + world, + client, + proposer, + keys, + context, + } + } + + impl Harness { + fn pubkey(&self) -> BlsPubkey { + self.world.state.validator(self.proposer).unwrap().pubkey + } + + /// The proposer's registration, signed by the client and sent through + /// this node. The relay checks the signature itself. + async fn register(&self) -> SignedValidatorRegistrationV1 { + let message = ValidatorRegistrationV1 { + fee_recipient: ExecutionAddress::repeat_byte(0x11), + gas_limit: TARGET_GAS_LIMIT, + timestamp: 1, + pubkey: self.pubkey(), + }; + let signature = { + let keys = self.keys.read().await; + self.context + .sign_validator_registration(&keys, &message) + .unwrap() + }; + let signed = SignedValidatorRegistrationV1 { message, signature }; + self.client + .register_validators(std::slice::from_ref(&signed)) + .await + .expect("the node takes the registration"); + signed + } + + async fn produce( + &self, + builder_boost_factor: Option, + ) -> ethlambda_validator::beacon_node::block_contents::ProducedBlock { + let epoch = compute_epoch_at_slot(self.world.slot); + // Signed under the head state's own fork rather than through the + // client's schedule: this world's state is not pinned to carry + // the schedule's fulu version (see `serve_fulu_pinned`), and the + // node verifies against the state, as the other proposal tests + // here do. + let randao_domain = get_domain( + &self.world.state, + ethlambda_types::beacon::constants::DOMAIN_RANDAO, + Some(epoch), + ); + let randao_reveal = sign_for( + self.proposer as usize, + compute_signing_root(epoch.hash_tree_root(), randao_domain), + ); + let request = BlockRequest { + slot: self.world.slot, + fork: ForkName::Fulu, + proposer_index: self.proposer, + randao_reveal, + graffiti: ethlambda_types::primitives::H256([0; 32]), + builder_boost_factor, + }; + self.client.produce_block(&request).await.unwrap() + } + + async fn sign( + &self, + produced: ethlambda_validator::beacon_node::block_contents::ProducedBlock, + ) -> Vec { + let domain = get_domain( + &self.world.state, + ethlambda_types::beacon::constants::DOMAIN_BEACON_PROPOSER, + Some(compute_epoch_at_slot(self.world.slot)), + ); + let signature = sign_for( + self.proposer as usize, + compute_signing_root(produced.block_root(), domain), + ); + produced.into_signed_ssz(signature) + } + } + + #[tokio::test] + async fn the_validator_client_can_propose_a_builder_block_through_this_node() { + let world = builder_world(false, SyncStatus::Synced).await; + let engine = fake_execution_client_paying("0x2a", false).await; + let h = harness(world, engine).await; + + let registered = h.register().await; + assert_eq!(h.world.relay.registrations(), [registered]); + + // Worth far less than the local payload (42 wei), so only the maximum + // factor makes it win: a factor that is dropped or ignored (the node's + // default is 100) leaves a full block. + h.world.offer(1, |_| {}); + let produced = h.produce(Some(u64::MAX)).await; + assert!( + produced.is_blinded(), + "the factor must reach the node and take the bid" + ); + + let body = h.sign(produced).await; + let published = h + .client + .publish_blinded_block(h.world.slot, ForkName::Fulu, &body) + .await + .expect("the node hands the block to its builder"); + assert_eq!(published, Published::HandedToBuilder); + + let signed = SignedBlindedBeaconBlock::from_ssz_bytes(&body).unwrap(); + assert_eq!(h.world.relay.blinded_blocks(), [signed]); + } + + #[tokio::test] + async fn without_builder_proposals_the_client_gets_a_full_block() { + let world = builder_world(false, SyncStatus::Synced).await; + let engine = fake_execution_client_paying("0x2a", false).await; + let h = harness(world, engine).await; + + // Registered, with a bid worth far more than the local payload: only + // the factor the client sends (0 for `None`) keeps the block local. + h.register().await; + h.world.offer(ETH, |_| {}); + let produced = h.produce(None).await; + assert!(!produced.is_blinded()); + assert!(h.world.relay.blinded_blocks().is_empty()); + assert_eq!( + h.world.relay.header_requests().len(), + 1, + "the node still asks the relay at a factor of 0" + ); + } + + #[tokio::test] + async fn a_relay_that_is_down_leaves_a_full_block() { + let world = builder_world(false, SyncStatus::Synced).await; + world.relay.answer_headers_with( + axum::http::StatusCode::SERVICE_UNAVAILABLE, + "application/json", + Vec::new(), + None, + ); + let engine = fake_execution_client_paying("0x2a", false).await; + let h = harness(world, engine).await; + + h.register().await; + h.world.offer(ETH, |_| {}); + let produced = h.produce(Some(100)).await; + assert!(!produced.is_blinded()); + } +} diff --git a/crates/validator/src/beacon_node/block_contents.rs b/crates/validator/src/beacon_node/block_contents.rs index c4d2806ca..9fbec02f0 100644 --- a/crates/validator/src/beacon_node/block_contents.rs +++ b/crates/validator/src/beacon_node/block_contents.rs @@ -21,7 +21,7 @@ //! //! The endpoints support it: `Accept: application/octet-stream` on //! `produceBlockV3`, `Content-Type: application/octet-stream` on -//! `publishBlockV2`. +//! `publishBlockV2` and `publishBlindedBlockV2`. //! //! # `BlockContents` is not a consensus container //! @@ -42,8 +42,20 @@ //! | Fork | Produced | Published | //! |---|---|---| //! | Deneb, Electra, Fulu | `BlockContents` | `SignedBlockContents` | +//! | Blinded (electra, fulu) | `BlindedBeaconBlock` | `SignedBlindedBeaconBlock` | //! | Gloas and later | `GloasBlockContents` (block and self-built envelope) or a bare block | bare signed block, then the envelope separately | //! +//! # Blinded blocks +//! +//! When the beacon node's builder wins the slot, `produceBlockV3` answers with +//! a [`BlindedBeaconBlock`] (`Eth-Execution-Payload-Blinded: true`): the block +//! carries the payload's header, and the builder reveals the payload once this +//! client has signed. No blobs travel with it, since the builder publishes +//! them. Its root is the full block's, so signing and the proposal guard work +//! as for any other block. Electra and fulu share one blinded container. +//! Electra blinded blocks are for third-party beacon nodes: an ethlambda node +//! serves blinded blocks for fulu only. +//! //! # Gloas //! //! The block no longer carries the payload (EIP-7732): it commits to a bid, and @@ -85,6 +97,7 @@ //! thirty-two blobs would fail to decode, in a client that had been correct for //! months. +use ethlambda_types::beacon::builder::{BlindedBeaconBlock, SignedBlindedBeaconBlock}; use ethlambda_types::beacon::containers::deneb::Blob; use ethlambda_types::beacon::containers::electra::{BeaconBlock, SignedBeaconBlock}; use ethlambda_types::beacon::containers::gloas; @@ -256,6 +269,10 @@ pub enum Contents { block: gloas::BeaconBlock, payload: Option>, }, + /// Electra or fulu, when a builder supplied the payload: the block carries + /// the payload's header, and the builder reveals the payload after this + /// client signs. No blobs travel with it; the builder publishes them. + Blinded { block: BlindedBeaconBlock }, } impl ProducedBlock { @@ -326,6 +343,45 @@ impl ProducedBlock { } } + /// Decode a blinded `produceBlockV3` body (`Eth-Execution-Payload-Blinded: + /// true`). Electra and fulu share one blinded container; every other fork is + /// refused, as [`Self::from_ssz`] refuses them. + pub fn from_blinded_ssz(fork: ForkName, bytes: &[u8]) -> crate::error::Result { + match fork { + ForkName::Electra | ForkName::Fulu => { + let block = BlindedBeaconBlock::from_ssz_bytes(bytes).map_err(|err| { + Error::Decode(format!( + "produced blinded block for fork {} did not decode: {err:?}", + fork.as_str() + )) + })?; + Ok(Self { + fork, + contents: Contents::Blinded { block }, + }) + } + // Named, as `from_ssz` names its arms, so a new fork forces a + // decision here instead of being refused by default. + ForkName::Phase0 + | ForkName::Altair + | ForkName::Bellatrix + | ForkName::Capella + | ForkName::Deneb + | ForkName::Gloas + | ForkName::Lean => Err(Error::InconsistentResponse(format!( + "beacon node produced a blinded block for fork {}; blinded blocks exist for \ + electra and fulu only", + fork.as_str() + ))), + } + } + + /// Whether a builder supplied the payload, so the block is published to + /// `blinded_blocks` and its fee recipient is the builder's. + pub fn is_blinded(&self) -> bool { + matches!(self.contents, Contents::Blinded { .. }) + } + /// Decode a gloas `produceBlockV4` body. `payload_included` is the node's /// `Eth-Execution-Payload-Included`, which selects between /// [`GloasBlockContents`] and a bare block: SSZ carries no tag, so a wrong @@ -365,6 +421,7 @@ impl ProducedBlock { block.slot } Contents::Gloas { block, .. } => block.slot, + Contents::Blinded { block } => block.slot, } } @@ -375,6 +432,7 @@ impl ProducedBlock { block.proposer_index } Contents::Gloas { block, .. } => block.proposer_index, + Contents::Blinded { block } => block.proposer_index, } } @@ -387,11 +445,13 @@ impl ProducedBlock { block.hash_tree_root() } Contents::Gloas { block, .. } => block.hash_tree_root(), + Contents::Blinded { block } => block.hash_tree_root(), } } /// Where the block pays its execution-layer rewards: the payload's fee - /// recipient before gloas, the bid's from it on. + /// recipient before gloas, the bid's from it on. For a blinded block, the + /// header's, which is the builder's address rather than the validator's. pub fn fee_recipient(&self) -> ethlambda_types::beacon::primitives::ExecutionAddress { match &self.contents { Contents::WithBlobProofs { block, .. } | Contents::WithCellProofs { block, .. } => { @@ -404,6 +464,7 @@ impl ProducedBlock { .message .fee_recipient } + Contents::Blinded { block } => block.body.execution_payload_header.fee_recipient, } } @@ -441,10 +502,12 @@ impl ProducedBlock { Contents::Gloas { payload, .. } => { payload.as_ref().map_or(0, |payload| payload.blobs.len()) } + Contents::Blinded { block } => block.body.blob_kzg_commitments.len(), } } - /// Attach `signature` and encode the body `publishBlockV2` expects. + /// Attach `signature` and encode the body `publishBlockV2` expects, or + /// `publishBlindedBlockV2` for a blinded block. /// /// Consumes the block: the proofs and blobs are moved into the published /// body rather than copied, and a blob is 128 KiB, so a six-blob block @@ -485,6 +548,11 @@ impl ProducedBlock { signature, } .to_ssz(), + Contents::Blinded { block } => SignedBlindedBeaconBlock { + message: block, + signature, + } + .to_ssz(), } } } @@ -775,6 +843,46 @@ mod tests { } } + #[test] + fn a_blinded_block_decodes_and_keeps_the_full_blocks_root() { + let full = empty_block_for(5, 7); + let blinded = BlindedBeaconBlock::from(full.clone()); + + let produced = + ProducedBlock::from_blinded_ssz(ForkName::Fulu, &blinded.to_ssz()).expect("decodes"); + + assert!(produced.is_blinded()); + assert_eq!(produced.slot(), 5); + assert_eq!(produced.proposer_index(), 7); + assert_eq!(produced.block_root(), full.hash_tree_root()); + assert_eq!(produced.blob_count(), 0); + } + + #[test] + fn a_signed_blinded_block_encodes_as_signed_blinded_beacon_block() { + let blinded = BlindedBeaconBlock::from(empty_block_for(5, 7)); + let produced = + ProducedBlock::from_blinded_ssz(ForkName::Electra, &blinded.to_ssz()).expect("ok"); + + let signed = produced.into_signed_ssz(BlsSignature([6; 96])); + let decoded = SignedBlindedBeaconBlock::from_ssz_bytes(&signed).expect("decodes"); + assert_eq!(decoded.message, blinded); + assert_eq!(decoded.signature, BlsSignature([6; 96])); + } + + #[test] + fn a_blinded_block_for_gloas_or_deneb_is_refused() { + let bytes = BlindedBeaconBlock::from(empty_block_for(5, 7)).to_ssz(); + for fork in [ForkName::Gloas, ForkName::Deneb] { + let err = ProducedBlock::from_blinded_ssz(fork, &bytes).expect_err("must refuse"); + assert!( + matches!(err, Error::InconsistentResponse(_)), + "{} gave {err:?}", + fork.as_str() + ); + } + } + #[test] fn a_truncated_body_is_a_decode_error_not_a_panic() { let bytes = contents_bytes(1, 1); diff --git a/crates/validator/src/beacon_node/fallback.rs b/crates/validator/src/beacon_node/fallback.rs index 8251f9559..04b4652e4 100644 --- a/crates/validator/src/beacon_node/fallback.rs +++ b/crates/validator/src/beacon_node/fallback.rs @@ -8,6 +8,7 @@ use std::future::Future; use std::pin::Pin; use async_trait::async_trait; +use ethlambda_types::beacon::builder::SignedValidatorRegistrationV1; use ethlambda_types::beacon::config::Config; use ethlambda_types::beacon::containers::altair; use ethlambda_types::beacon::containers::gloas; @@ -39,12 +40,19 @@ type BoxFuture<'a, T> = Pin + Send + 'a>>; /// first one that succeeds. pub struct FallbackBeaconNode { nodes: Vec, + /// The node that produced the latest blinded block, by slot. A blinded + /// block can only be published where it was produced: that node's builder + /// holds the payload, and that node claimed the slot. + blinded_producer: std::sync::Mutex>, } impl FallbackBeaconNode { pub fn new(nodes: Vec) -> Self { assert!(!nodes.is_empty(), "at least one beacon node is required"); - Self { nodes } + Self { + nodes, + blinded_producer: std::sync::Mutex::new(None), + } } /// Try each node in order, returning the first success. @@ -68,13 +76,23 @@ impl FallbackBeaconNode { /// both `&self` and the future to one named `'p` lets the compiler unify /// it with whatever the call site's shortest borrow actually is. async fn try_each<'p, T, F>(&'p self, what: &'static str, call: F) -> Result + where + F: Fn(&'p B) -> BoxFuture<'p, Result>, + { + self.try_each_at(what, call).await.map(|(_, value)| value) + } + + /// [`Self::try_each`], also returning the position of the node that + /// 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>, { let mut last = None; for (position, node) in self.nodes.iter().enumerate() { match call(node).await { - Ok(value) => return Ok(value), + Ok(value) => return Ok((position, value)), Err(err) => { // Debug, not warn: with more than one node configured, an // outage on one of them fires this every slot for as long @@ -112,7 +130,18 @@ impl FallbackBeaconNode { /// unreachable node must not stop the reachable ones being told. A failure /// is logged per node so a partially-registered client is visible rather /// than silent. - async fn try_all<'p, F>(&'p self, what: &'static str, call: F) -> Result<()> + /// + /// `node_effect` says what a refusing node costs (said once per refusal); + /// `all_failed` says what it costs when none accepts, or is `None` when the + /// caller reports that itself, so a duty that did not depend on the call is + /// not claimed missed. + async fn try_all<'p, F>( + &'p self, + what: &'static str, + node_effect: &'static str, + all_failed: Option<&'static str>, + call: F, + ) -> Result<()> where F: Fn(&'p B) -> BoxFuture<'p, Result<()>>, { @@ -122,7 +151,7 @@ impl FallbackBeaconNode { match call(node).await { Ok(()) => accepted += 1, Err(err) => { - warn!(%what, position, %err, "Beacon node did not accept; it will be unprepared for this duty"); + warn!(%what, position, %err, node_effect, "Beacon node did not accept"); last = Some(err); } } @@ -133,7 +162,9 @@ impl FallbackBeaconNode { let detail = last .map(|err| err.to_string()) .unwrap_or_else(|| "no nodes configured".to_string()); - warn!(%what, %detail, "No configured beacon node accepted; duty likely missed"); + if let Some(all_failed) = all_failed { + warn!(%what, %detail, all_failed, "No configured beacon node accepted"); + } Err(Error::AllBeaconNodesFailed(detail)) } } @@ -230,9 +261,19 @@ impl BeaconNodeApi for FallbackBeaconNode { .await } + /// Remembers which node answered when the block is blinded, and forgets + /// it when it is not: the record is for the latest block only, so a full + /// block never leaves a stale claim that a blinded publish could match. async fn produce_block(&self, request: &BlockRequest) -> Result { - self.try_each("produce_block", |node| node.produce_block(request)) - .await + let (position, block) = self + .try_each_at("produce_block", |node| node.produce_block(request)) + .await?; + let record = block.is_blinded().then(|| (block.slot(), position)); + *self + .blinded_producer + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) = record; + Ok(block) } /// First node that accepts it wins, rather than a broadcast to every node. @@ -253,6 +294,54 @@ impl BeaconNodeApi for FallbackBeaconNode { .await } + /// Only on the node that produced the block, with no failover. + /// + /// Unlike [`Self::publish_block`], offering the block to another node + /// would not reach the network: only the producing node's builder holds + /// the payload the header commits to, and only that node claimed the slot + /// with it. A second node would at best refuse it and at worst hand a + /// signed block to a builder that never saw the bid. So if the producing + /// node fails, the publish fails, and the answer is returned unchanged so + /// the caller can tell a refusal from an unconfirmed send. The proposal + /// guard has already recorded the slot, so nothing retries it. + async fn publish_blinded_block( + &self, + slot: Slot, + fork: ForkName, + body: &[u8], + ) -> Result { + let record = *self + .blinded_producer + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + match record { + Some((produced_slot, position)) if produced_slot == slot => { + self.nodes[position] + .publish_blinded_block(slot, fork, body) + .await + } + _ => Err(Error::InconsistentResponse(format!( + "no beacon node produced a blinded block for slot {slot}" + ))), + } + } + + /// To every node, like the other calls that install state on a node: a + /// node that never heard the registrations cannot serve a builder bid for + /// these validators after a failover. + async fn register_validators( + &self, + registrations: &[SignedValidatorRegistrationV1], + ) -> Result<()> { + self.try_all( + "register_validators", + "it will not hold these registrations", + None, + |node| node.register_validators(registrations), + ) + .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( @@ -349,9 +438,12 @@ impl BeaconNodeApi for FallbackBeaconNode { /// that was never told where to pay builds a payload paying somewhere else, /// and it is the node this client falls over to that needs telling most. async fn prepare_beacon_proposer(&self, preparations: &[ProposerPreparationDto]) -> Result<()> { - self.try_all("prepare_beacon_proposer", |node| { - node.prepare_beacon_proposer(preparations) - }) + self.try_all( + "prepare_beacon_proposer", + "it will be unprepared for this duty", + Some("duty likely missed"), + |node| node.prepare_beacon_proposer(preparations), + ) .await } @@ -360,9 +452,12 @@ impl BeaconNodeApi for FallbackBeaconNode { /// the votes an aggregate is folded from, so a node that never received one /// answers 404 for every committee this client asks about. async fn subscribe_committees(&self, subscriptions: &[CommitteeSubscriptionDto]) -> Result<()> { - self.try_all("subscribe_committees", |node| { - node.subscribe_committees(subscriptions) - }) + self.try_all( + "subscribe_committees", + "it will be unprepared for this duty", + Some("duty likely missed"), + |node| node.subscribe_committees(subscriptions), + ) .await } @@ -423,9 +518,12 @@ impl BeaconNodeApi for FallbackBeaconNode { &self, subscriptions: &[SyncCommitteeSubscriptionDto], ) -> Result<()> { - self.try_all("subscribe_sync_committees", |node| { - node.subscribe_sync_committees(subscriptions) - }) + self.try_all( + "subscribe_sync_committees", + "it will be unprepared for this duty", + Some("duty likely missed"), + |node| node.subscribe_sync_committees(subscriptions), + ) .await } } @@ -601,6 +699,7 @@ mod tests { proposer_index, randao_reveal: BlsSignature([0; 96]), graffiti: Bytes32::default(), + builder_boost_factor: None, } } @@ -850,4 +949,158 @@ mod tests { "the healthy second node must be found" ); } + + fn blinded_request(slot: Slot, proposer_index: ValidatorIndex) -> BlockRequest { + BlockRequest { + builder_boost_factor: Some(100), + ..request(slot, proposer_index) + } + } + + fn blinded_node() -> MockBeaconNode { + MockBeaconNode::new().with_block(5, 7).with_blinded_block() + } + + /// Node 0 cannot produce, node 1 can and answers blinded: the block must go + /// back to node 1, and node 0 must never see it. + #[tokio::test] + async fn a_blinded_block_is_published_where_it_was_produced() { + let first = MockBeaconNode::new().failing_call("produce_block", "down"); + let fallback = FallbackBeaconNode::new(vec![first, blinded_node()]); + + let produced = fallback + .produce_block(&blinded_request(5, 7)) + .await + .expect("node 1 produces"); + assert!(produced.is_blinded()); + + let published = fallback + .publish_blinded_block(5, ForkName::Electra, &[1, 2, 3]) + .await + .expect("published"); + assert_eq!(published, Published::HandedToBuilder); + + let first_published = fallback.nodes[0].published_blinded.lock().expect("lock"); + let second_published = fallback.nodes[1].published_blinded.lock().expect("lock"); + assert!(first_published.is_empty(), "node 0 never produced it"); + assert_eq!( + *second_published, + vec![(5, ForkName::Electra, vec![1, 2, 3])] + ); + } + + /// The producing node failing the publish is the end of it: another node's + /// builder never saw the bid, so it must not be offered the block. + #[tokio::test] + async fn a_blinded_publish_does_not_fail_over() { + let second = blinded_node().with_unconfirmed_blinded_publish("relay timed out"); + let first = MockBeaconNode::new().failing_call("produce_block", "down"); + let fallback = FallbackBeaconNode::new(vec![first, second]); + + fallback + .produce_block(&blinded_request(5, 7)) + .await + .expect("node 1 produces"); + let err = fallback + .publish_blinded_block(5, ForkName::Electra, &[9]) + .await + .expect_err("node 1 failed to confirm"); + assert!( + matches!(err, Error::BlindedUnconfirmed { .. }), + "got {err:?}" + ); + assert!( + fallback.nodes[0] + .published_blinded + .lock() + .expect("lock") + .is_empty(), + "no failover to node 0" + ); + } + + #[tokio::test] + async fn a_blinded_publish_for_another_slot_is_refused() { + let fallback = FallbackBeaconNode::new(vec![blinded_node()]); + fallback + .produce_block(&blinded_request(5, 7)) + .await + .expect("produces"); + + let err = fallback + .publish_blinded_block(6, ForkName::Electra, &[1]) + .await + .expect_err("nothing was produced for slot 6"); + assert!(matches!(err, Error::InconsistentResponse(_)), "got {err:?}"); + assert!( + fallback.nodes[0] + .published_blinded + .lock() + .expect("lock") + .is_empty() + ); + } + + /// A full block produced afterwards clears the claim, so an old blinded + /// block's slot cannot be published against a node that no longer holds it. + #[tokio::test] + async fn a_full_block_clears_the_blinded_record() { + let fallback = FallbackBeaconNode::new(vec![blinded_node()]); + fallback + .produce_block(&blinded_request(5, 7)) + .await + .expect("blinded"); + fallback + .produce_block(&request(5, 7)) + .await + .expect("full, since no factor was sent"); + + fallback + .publish_blinded_block(5, ForkName::Electra, &[1]) + .await + .expect_err("the record is gone"); + } + + #[tokio::test] + async fn registrations_reach_every_node() { + use ethlambda_types::beacon::builder::{ + SignedValidatorRegistrationV1, ValidatorRegistrationV1, + }; + use ethlambda_types::beacon::primitives::{BlsPubkey, BlsSignature, H160}; + + let registration = SignedValidatorRegistrationV1 { + message: ValidatorRegistrationV1 { + fee_recipient: H160([1; 20]), + gas_limit: 30_000_000, + timestamp: 5, + pubkey: BlsPubkey([2; 48]), + }, + signature: BlsSignature([3; 96]), + }; + let fallback = FallbackBeaconNode::new(vec![ + MockBeaconNode::new(), + MockBeaconNode::failing("middle down"), + MockBeaconNode::new(), + ]); + + fallback + .register_validators(std::slice::from_ref(®istration)) + .await + .expect("two of three accepted"); + + for position in [0, 2] { + assert_eq!( + *fallback.nodes[position].registrations.lock().expect("lock"), + vec![registration.clone()], + "node {position}" + ); + } + assert!( + fallback.nodes[1] + .registrations + .lock() + .expect("lock") + .is_empty() + ); + } } diff --git a/crates/validator/src/beacon_node/http.rs b/crates/validator/src/beacon_node/http.rs index 013964c74..599119890 100644 --- a/crates/validator/src/beacon_node/http.rs +++ b/crates/validator/src/beacon_node/http.rs @@ -23,6 +23,7 @@ use std::time::Duration; use async_trait::async_trait; +use ethlambda_types::beacon::builder::SignedValidatorRegistrationV1; use ethlambda_types::beacon::config::Config; use ethlambda_types::beacon::containers::altair; use ethlambda_types::beacon::containers::electra::Attestation; @@ -60,6 +61,12 @@ use crate::error::{BeaconNodeFailure, Error, Result}; /// a stalled request could then outlive the slot it was serving. const REQUEST_TIMEOUT: Duration = Duration::from_secs(8); +/// How long `register_validator` may take. Longer than [`REQUEST_TIMEOUT`] +/// because an ethlambda node forwards the registrations to its relay and waits +/// up to 10 s for the answer before replying; a client that gave up at 8 s +/// would report a failure for a registration the node went on to install. +const REGISTRATION_TIMEOUT: Duration = Duration::from_secs(12); + /// What an SSZ answer carries besides its bytes: the fork, which only a header /// can say, and the two booleans the block endpoints use to say what the bytes /// are. @@ -146,12 +153,28 @@ impl HttpBeaconNode { path: &str, body: &B, consensus_version: Option<&str>, + ) -> Result<()> { + self.post_no_content_within(path, body, consensus_version, None) + .await + } + + /// [`Self::post_no_content`] with `timeout` in place of the client's + /// default, when given. + async fn post_no_content_within( + &self, + path: &str, + body: &B, + consensus_version: Option<&str>, + timeout: Option, ) -> Result<()> { let url = format!("{}{path}", self.base_url); let mut request = self.client.post(&url).json(body); if let Some(version) = consensus_version { request = request.header("Eth-Consensus-Version", version); } + if let Some(timeout) = timeout { + request = request.timeout(timeout); + } let response = request.send().await.map_err(|err| Error::BeaconNode { url: url.clone(), failure: BeaconNodeFailure::classify(&err), @@ -168,6 +191,30 @@ impl HttpBeaconNode { }) } + /// POST an SSZ body with the fork it is for, returning the URL beside the + /// outcome so each caller maps a transport failure its own way: a full + /// block's is an ordinary error, a blinded block's may be unconfirmed. + async fn post_ssz_body( + &self, + path: &str, + fork: ForkName, + body: &[u8], + ) -> ( + String, + std::result::Result, + ) { + let url = format!("{}{path}", self.base_url); + let sent = self + .client + .post(&url) + .header("Eth-Consensus-Version", fork.as_str()) + .header(reqwest::header::CONTENT_TYPE, "application/octet-stream") + .body(body.to_vec()) + .send() + .await; + (url, sent) + } + /// Fetch a body as SSZ, returning the fork the node named alongside it. /// /// `Accept` names SSZ alone, deliberately narrower than the specification's @@ -593,8 +640,9 @@ impl BeaconNodeApi for HttpBeaconNode { Ok(data) } - /// `builder_boost_factor=0` is how this client says it does not do the - /// builder flow. + /// `builder_boost_factor` is how this client says whether it does the + /// builder flow: the request's factor when builder proposals are on, `0` + /// when they are off (`None`). /// /// The parameter is a bid comparison: the node returns the local execution /// payload when its value is at least `factor / 100` of the builder's, so @@ -607,19 +655,21 @@ impl BeaconNodeApi for HttpBeaconNode { /// answer with a blinded block. The one unconditional guarantee is the /// other half: a node with no builder configured MUST return a full block. /// - /// A blinded answer is therefore rejected below rather than assumed away. - /// This client cannot publish what it cannot unblind, so signing one would - /// burn the slot's proposal guard entry for a block that can never be - /// sent. + /// A blinded answer is therefore never assumed away. With builder proposals + /// off (`None`) it is rejected: this client would not publish it through + /// `blinded_blocks`, so signing one would burn the slot's proposal guard + /// entry for a block that can never be sent. With them on it is decoded as + /// a blinded block, which `publish_blinded_block` sends. async fn produce_block(&self, request: &BlockRequest) -> Result { if request.fork >= ForkName::Gloas { return self.produce_gloas_block(request).await; } let path = format!( - "/eth/v3/validator/blocks/{}?randao_reveal={}&graffiti={}&builder_boost_factor=0", + "/eth/v3/validator/blocks/{}?randao_reveal={}&graffiti={}&builder_boost_factor={}", request.slot, encode_hex(&request.randao_reveal.0), encode_hex(&request.graffiti.0), + request.builder_boost_factor.unwrap_or(0), ); let SszResponse { fork, @@ -639,15 +689,19 @@ impl BeaconNodeApi for HttpBeaconNode { .to_string(), ) })?; - if blinded { + if blinded && request.builder_boost_factor.is_none() { return Err(Error::InconsistentResponse(format!( - "node produced a blinded block for slot {}; this client does not implement the \ - builder flow and cannot publish one", + "node produced a blinded block for slot {}; builder proposals are off, so this \ + client cannot publish one", request.slot ))); } - let block = ProducedBlock::from_ssz(fork, &body)?; + let block = if blinded { + ProducedBlock::from_blinded_ssz(fork, &body)? + } else { + ProducedBlock::from_ssz(fork, &body)? + }; // Enforced here rather than at the call site so failover works: see the // contract on `BeaconNodeApi::produce_block`. validate_produced_block(request, &block)?; @@ -657,21 +711,14 @@ impl BeaconNodeApi for HttpBeaconNode { /// 202 is not an error and not a clean success, so it is neither mapped to /// `Err` nor flattened into `Ok(())`. See [`Published`]. async fn publish_block(&self, fork: ForkName, body: &[u8]) -> Result { - let path = "/eth/v2/beacon/blocks"; - let url = format!("{}{path}", self.base_url); - let response = self - .client - .post(&url) - .header("Eth-Consensus-Version", fork.as_str()) - .header(reqwest::header::CONTENT_TYPE, "application/octet-stream") - .body(body.to_vec()) - .send() - .await - .map_err(|err| Error::BeaconNode { - url: url.clone(), - failure: BeaconNodeFailure::classify(&err), - detail: err.to_string(), - })?; + let (url, sent) = self + .post_ssz_body("/eth/v2/beacon/blocks", fork, body) + .await; + let response = sent.map_err(|err| Error::BeaconNode { + url: url.clone(), + failure: BeaconNodeFailure::classify(&err), + detail: err.to_string(), + })?; let status = response.status(); if status == StatusCode::ACCEPTED { @@ -687,6 +734,79 @@ impl BeaconNodeApi for HttpBeaconNode { }) } + /// Maps the answer as the trait's contract says: 2xx is handed to the + /// builder (202 is the node's broadcast-without-import, as for a full + /// block), and a 4xx is a definite refusal. + /// + /// Two cases that look like failures are also definite, because the node + /// cannot have acted on the block: a connection error (the request never + /// left) and a 503 (the node is syncing, and refuses before it hands + /// anything to a builder). Every other 5xx, and any failure after the + /// request was sent (a timeout, a reset), is [`Error::BlindedUnconfirmed`], + /// since the relay may still publish. + async fn publish_blinded_block( + &self, + _slot: Slot, + fork: ForkName, + body: &[u8], + ) -> Result { + let (url, sent) = self + .post_ssz_body("/eth/v2/beacon/blinded_blocks", fork, body) + .await; + let response = sent.map_err(|err| { + if err.is_connect() { + Error::BeaconNode { + url: url.clone(), + failure: BeaconNodeFailure::classify(&err), + detail: err.to_string(), + } + } else { + Error::BlindedUnconfirmed { + status: None, + detail: format!("{url}: {err}"), + } + } + })?; + + let status = response.status(); + if status == StatusCode::ACCEPTED { + return Ok(Published::BroadcastNotImported); + } + if status.is_success() { + return Ok(Published::HandedToBuilder); + } + let body = response.text().await.unwrap_or_default(); + if status.is_server_error() && status != StatusCode::SERVICE_UNAVAILABLE { + return Err(Error::BlindedUnconfirmed { + status: Some(status.as_u16()), + detail: format!("{url} answered {status}: {body}"), + }); + } + Err(Error::BeaconNodeStatus { + status: status.as_u16(), + body, + }) + } + + /// JSON, an empty list never reaching the wire. + async fn register_validators( + &self, + registrations: &[SignedValidatorRegistrationV1], + ) -> Result<()> { + if registrations.is_empty() { + return Ok(()); + } + // Its own timeout: the node waits up to 10 s on the relay before it + // answers, which is longer than every other request's default. + self.post_no_content_within( + "/eth/v1/validator/register_validator", + ®istrations, + None, + Some(REGISTRATION_TIMEOUT), + ) + .await + } + async fn execution_payload_envelope( &self, slot: Slot, @@ -1189,6 +1309,243 @@ mod tests { assert_eq!(value["builders"], serde_json::json!([])); } + /// A Beacon API double that answers every route the same way: whatever + /// the handler sees is pushed to `seen` as (uri, body, request headers), + /// and the answer is `status` with `headers` and `reply` as the body. + type Seen = std::sync::Arc, axum::http::HeaderMap)>>>; + type Canned = (Seen, u16, Vec<(&'static str, &'static str)>, Vec); + + struct Served { + node: HttpBeaconNode, + seen: Seen, + } + + async fn serve( + status: u16, + headers: Vec<(&'static str, &'static str)>, + reply: Vec, + ) -> Served { + use axum::body::Bytes; + use axum::extract::State; + use axum::http::Uri; + use axum::response::IntoResponse; + + let seen: Seen = Default::default(); + let state = (seen.clone(), status, headers, reply); + let handler = |State(state): State, + uri: Uri, + request_headers: axum::http::HeaderMap, + body: Bytes| async move { + let (seen, status, headers, reply) = state; + seen.lock() + .expect("lock") + .push((uri.to_string(), body.to_vec(), request_headers)); + let mut response = ( + axum::http::StatusCode::from_u16(status).expect("a status"), + reply, + ) + .into_response(); + for (name, value) in headers { + response + .headers_mut() + .insert(name, axum::http::HeaderValue::from_static(value)); + } + response + }; + let app = axum::Router::new().fallback(handler).with_state(state); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("binds"); + let address = listener.local_addr().expect("address"); + tokio::spawn(async move { axum::serve(listener, app).await }); + Served { + node: HttpBeaconNode::new(format!("http://{address}")).expect("builds"), + seen, + } + } + + fn block_request(factor: Option) -> BlockRequest { + BlockRequest { + slot: 5, + fork: ForkName::Fulu, + proposer_index: 7, + randao_reveal: Default::default(), + graffiti: Default::default(), + builder_boost_factor: factor, + } + } + + const SSZ_HEADERS: [(&str, &str); 3] = [ + ("content-type", "application/octet-stream"), + ("eth-consensus-version", "fulu"), + ("eth-execution-payload-blinded", "true"), + ]; + + fn blinded_bytes() -> Vec { + use libssz::SszEncode as _; + ethlambda_types::beacon::builder::BlindedBeaconBlock::from( + crate::beacon_node::block_contents::empty_block_for(5, 7), + ) + .to_ssz() + } + + #[tokio::test] + async fn the_boost_factor_is_zero_without_builder_proposals_and_the_factor_with() { + for (factor, expected) in [ + (None, "builder_boost_factor=0"), + (Some(250), "builder_boost_factor=250"), + ] { + let served = serve(500, vec![], vec![]).await; + let _ = served.node.produce_block(&block_request(factor)).await; + let seen = served.seen.lock().expect("lock"); + assert_eq!(seen.len(), 1); + assert!( + seen[0].0.contains(expected), + "{} lacks {expected}", + seen[0].0 + ); + } + } + + #[tokio::test] + async fn a_blinded_answer_is_refused_without_builder_proposals() { + let served = serve(200, SSZ_HEADERS.to_vec(), blinded_bytes()).await; + let err = served + .node + .produce_block(&block_request(None)) + .await + .expect_err("a blinded block cannot be published here"); + assert!(matches!(err, Error::InconsistentResponse(_)), "got {err:?}"); + } + + #[tokio::test] + async fn a_blinded_answer_decodes_as_blinded_with_builder_proposals() { + let served = serve(200, SSZ_HEADERS.to_vec(), blinded_bytes()).await; + let produced = served + .node + .produce_block(&block_request(Some(100))) + .await + .expect("decodes"); + assert!(produced.is_blinded()); + assert_eq!(produced.slot(), 5); + assert_eq!(produced.proposer_index(), 7); + } + + #[tokio::test] + async fn registrations_are_posted_as_json_that_round_trips() { + use ethlambda_types::beacon::builder::{ + SignedValidatorRegistrationV1, ValidatorRegistrationV1, + }; + use ethlambda_types::beacon::primitives::{BlsPubkey, BlsSignature, H160}; + + let registrations = vec![SignedValidatorRegistrationV1 { + message: ValidatorRegistrationV1 { + fee_recipient: H160([0xab; 20]), + gas_limit: 36_000_000, + timestamp: 1_700_000_000, + pubkey: BlsPubkey([3; 48]), + }, + signature: BlsSignature([4; 96]), + }]; + let served = serve(200, vec![], vec![]).await; + served + .node + .register_validators(®istrations) + .await + .expect("accepted"); + + let seen = served.seen.lock().expect("lock"); + assert_eq!(seen.len(), 1); + assert!(seen[0].0.ends_with("/eth/v1/validator/register_validator")); + let decoded: Vec = + serde_json::from_slice(&seen[0].1).expect("the body is the JSON a relay takes"); + assert_eq!(decoded, registrations); + } + + #[tokio::test] + async fn an_empty_registration_list_never_reaches_the_wire() { + let served = serve(200, vec![], vec![]).await; + served.node.register_validators(&[]).await.expect("a no-op"); + assert!(served.seen.lock().expect("lock").is_empty()); + } + + #[tokio::test] + async fn a_blinded_publish_maps_each_status() { + for (status, expected) in [ + (200, Published::HandedToBuilder), + (202, Published::BroadcastNotImported), + ] { + let served = serve(status, vec![], vec![]).await; + let published = served + .node + .publish_blinded_block(5, ForkName::Fulu, &[1, 2, 3]) + .await + .expect("accepted"); + assert_eq!(published, expected, "status {status}"); + let seen = served.seen.lock().expect("lock"); + assert!(seen[0].0.ends_with("/eth/v2/beacon/blinded_blocks")); + assert_eq!(seen[0].1, vec![1, 2, 3]); + assert_eq!(seen[0].2["eth-consensus-version"], "fulu"); + assert_eq!(seen[0].2["content-type"], "application/octet-stream"); + } + + let served = serve(400, vec![], b"bad".to_vec()).await; + let err = served + .node + .publish_blinded_block(5, ForkName::Fulu, &[1]) + .await + .expect_err("a refusal"); + assert!( + matches!(err, Error::BeaconNodeStatus { status: 400, .. }), + "got {err:?}" + ); + + let served = serve(500, vec![], b"boom".to_vec()).await; + let err = served + .node + .publish_blinded_block(5, ForkName::Fulu, &[1]) + .await + .expect_err("unconfirmed"); + assert!( + matches!( + err, + Error::BlindedUnconfirmed { + status: Some(500), + .. + } + ), + "got {err:?}" + ); + } + + /// The request never left, so the relay cannot have the block: a definite + /// failure, not an unconfirmed one. + #[tokio::test] + async fn a_blinded_publish_that_cannot_connect_is_a_definite_failure() { + // Nothing listens on port 1. + let node = HttpBeaconNode::new("http://127.0.0.1:1").expect("builds"); + let err = node + .publish_blinded_block(5, ForkName::Fulu, &[1]) + .await + .expect_err("no answer"); + assert!(matches!(err, Error::BeaconNode { .. }), "got {err:?}"); + } + + /// A syncing node refuses before it hands anything to a builder. + #[tokio::test] + async fn a_blinded_publish_to_a_syncing_node_is_a_definite_failure() { + let served = serve(503, vec![], b"syncing".to_vec()).await; + let err = served + .node + .publish_blinded_block(5, ForkName::Fulu, &[1]) + .await + .expect_err("refused"); + assert!( + matches!(err, Error::BeaconNodeStatus { status: 503, .. }), + "got {err:?}" + ); + } + #[test] fn a_boolean_header_is_read_strictly() { assert_eq!( diff --git a/crates/validator/src/beacon_node/mock.rs b/crates/validator/src/beacon_node/mock.rs index fe4a952b9..409e3aedf 100644 --- a/crates/validator/src/beacon_node/mock.rs +++ b/crates/validator/src/beacon_node/mock.rs @@ -5,6 +5,7 @@ use std::collections::HashMap; use std::sync::Mutex; use async_trait::async_trait; +use ethlambda_types::beacon::builder::{BlindedBeaconBlock, SignedValidatorRegistrationV1}; use ethlambda_types::beacon::config::Config; use ethlambda_types::beacon::containers::altair; use ethlambda_types::beacon::containers::electra::{Attestation, SignedAggregateAndProof}; @@ -79,6 +80,28 @@ pub struct MockBeaconNode { /// What `publish_block` reports. Defaults to `Imported`; a test that cares /// about the 202 path sets it. pub publish_outcome: Option, + /// When set, `produce_block` answers a pre-gloas request that carries a + /// `builder_boost_factor` with a blinded block (for the slot and proposer + /// `produces_block` names), as a node whose builder won the slot does. A + /// request with no factor still gets a full block, as the contract on + /// `BeaconNodeApi::produce_block` requires. + pub produces_blinded: bool, + /// Every registration `register_validators` received, in call order. + pub registrations: Mutex>, + /// The size of each `register_validators` call, in call order, so a test + /// can see how a send was batched. + pub registration_batches: Mutex>, + /// Every blinded block published, as (slot, fork, body). + pub published_blinded: Mutex)>>, + /// What `publish_blinded_block` reports. `None` is `Ok(HandedToBuilder)`; + /// `Err(detail)` is a `BlindedUnconfirmed` carrying `detail`. + pub blinded_publish_outcome: Option>, + /// When set, `publish_blinded_block` answers with this status as a node + /// that definitely refused the block (a 4xx) does: `BeaconNodeStatus`. + pub blinded_publish_refusal: Option, + /// How long `publish_blinded_block` takes before it records the block and + /// answers, as a slow relay makes the node. + pub blinded_publish_delay: Option, /// What the test asserts against. pub submitted: Mutex>, @@ -258,6 +281,33 @@ impl MockBeaconNode { self } + /// Answer a `produce_block` that carries a `builder_boost_factor` with a + /// blinded block, for the slot and proposer `with_block` names. + pub fn with_blinded_block(mut self) -> Self { + self.produces_blinded = true; + self + } + + /// Make `publish_blinded_block` fail as a node that did not confirm the + /// block, carrying `detail`. + pub fn with_unconfirmed_blinded_publish(mut self, detail: &str) -> Self { + self.blinded_publish_outcome = Some(Err(detail.to_string())); + self + } + + /// 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); + self + } + + /// Make `publish_blinded_block` fail as a node that definitely refused the + /// block, with `status`. + pub fn with_refused_blinded_publish(mut self, status: u16) -> Self { + self.blinded_publish_refusal = Some(status); + self + } + /// Answer `produce_block` at a gloas slot with a bare gloas block whose /// bid names `builder_index`, with its self-built envelope attached when /// `include_payload` is set. @@ -640,6 +690,16 @@ impl BeaconNodeApi for MockBeaconNode { failure: BeaconNodeFailure::Request, detail: "no block set".into(), })?; + if self.produces_blinded && request.builder_boost_factor.is_some() { + let block = ProducedBlock { + fork: ForkName::Electra, + contents: Contents::Blinded { + block: BlindedBeaconBlock::from(empty_block_for(slot, proposer_index)), + }, + }; + crate::beacon_node::validate_produced_block(request, &block)?; + return Ok(block); + } // Electra, matching the block shape `empty_block_for` builds. A test // that needs a fork mismatch constructs one itself rather than getting // it by accident here. @@ -664,6 +724,59 @@ impl BeaconNodeApi for MockBeaconNode { Ok(self.publish_outcome.unwrap_or(Published::Imported)) } + async fn publish_blinded_block( + &self, + slot: Slot, + fork: ForkName, + body: &[u8], + ) -> Result { + // `failures` and `fail_everything` stand for a node that could not be + // reached or answered badly: over HTTP that is an unconfirmed publish, + // never a definite refusal (see `with_refused_blinded_publish`). + self.guard("publish_blinded_block") + .map_err(|err| Error::BlindedUnconfirmed { + status: None, + detail: err.to_string(), + })?; + if let Some(status) = self.blinded_publish_refusal { + return Err(Error::BeaconNodeStatus { + status, + body: "refused".to_string(), + }); + } + if let Some(delay) = self.blinded_publish_delay { + tokio::time::sleep(delay).await; + } + self.published_blinded + .lock() + .expect("lock") + .push((slot, fork, body.to_vec())); + match &self.blinded_publish_outcome { + None => Ok(Published::HandedToBuilder), + Some(Ok(published)) => Ok(*published), + Some(Err(detail)) => Err(Error::BlindedUnconfirmed { + status: None, + detail: detail.clone(), + }), + } + } + + async fn register_validators( + &self, + registrations: &[SignedValidatorRegistrationV1], + ) -> Result<()> { + self.guard("register_validators")?; + self.registration_batches + .lock() + .expect("lock") + .push(registrations.len()); + self.registrations + .lock() + .expect("lock") + .extend(registrations.iter().cloned()); + 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 c689d2a1d..ed292c5f4 100644 --- a/crates/validator/src/beacon_node/mod.rs +++ b/crates/validator/src/beacon_node/mod.rs @@ -6,6 +6,7 @@ //! (a reorg invalidating duties, a node failing mid-slot) untestable. use async_trait::async_trait; +use ethlambda_types::beacon::builder::SignedValidatorRegistrationV1; use ethlambda_types::beacon::config::Config; use ethlambda_types::beacon::containers::shared::AttestationData; use ethlambda_types::beacon::fork::ForkName; @@ -130,6 +131,11 @@ pub struct BlockRequest { pub randao_reveal: BlsSignature, /// Thirty-two bytes consensus never reads. 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. + pub builder_boost_factor: Option, } /// What became of a published block. @@ -145,6 +151,14 @@ pub enum Published { Imported, /// 202: broadcast, but the node could not import it. BroadcastNotImported, + /// 200 from `blinded_blocks`: the node handed the signed blinded block to + /// its builder, which publishes the block and reveals the payload. Nothing + /// says the node imported it. + HandedToBuilder, + /// The node failed to confirm the blinded block (5xx or no answer). The + /// relay may still have published it; see `Error::BlindedUnconfirmed`. + /// Produced only by `ProposalService`, never by an implementation. + BuilderUnconfirmed, } /// Check that `block` is the block that was asked for. @@ -351,12 +365,13 @@ pub trait BeaconNodeApi: Send + Sync { /// returning, so that a node answering about the wrong slot or naming the /// wrong proposer is failed over from rather than signed for. /// - /// # Contract: never a blinded block + /// # Contract: a blinded block only when builder proposals are on /// - /// This client does not implement the builder flow, so it cannot publish a - /// blinded block and must not sign one. An implementation must ask for an - /// unblinded block and reject a blinded answer rather than returning - /// something the caller will sign and then be unable to send. + /// A blinded block is returned only when `request.builder_boost_factor` is + /// `Some`, since only then does the caller publish one through + /// [`Self::publish_blinded_block`]. With `None` an implementation must ask + /// for an unblinded block and reject a blinded answer rather than + /// returning something the caller will sign and then be unable to send. async fn produce_block(&self, request: &BlockRequest) -> Result; /// Publish an already-signed block, given the SSZ body and the fork it was @@ -372,6 +387,35 @@ pub trait BeaconNodeApi: Send + Sync { /// proposal, whether or not the first one succeeded. async fn publish_block(&self, fork: ForkName, body: &[u8]) -> Result; + /// Publish a signed blinded block, produced at `slot` under `fork`, to + /// `POST /eth/v2/beacon/blinded_blocks`. The node hands it to its builder. + /// + /// # Contract: the node that produced it, and no other + /// + /// Only the producing node's builder holds the payload, and only that node + /// claimed the slot. Failover must not offer it elsewhere. + /// + /// # Contract: which error means what + /// + /// A definite refusal (4xx) is an `Err` other than + /// `Error::BlindedUnconfirmed`: the block was not published. A 5xx or a + /// transport failure is `Error::BlindedUnconfirmed`, because the relay may + /// still publish the block. + async fn publish_blinded_block( + &self, + slot: Slot, + fork: ForkName, + body: &[u8], + ) -> Result; + + /// Send signed builder registrations to the node, which forwards them to its + /// builder. JSON, since a node may forward the body unchanged to MEV-Boost. + /// State installed on the node's builder, so failover sends it to every node. + async fn register_validators( + &self, + registrations: &[SignedValidatorRegistrationV1], + ) -> 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/error.rs b/crates/validator/src/error.rs index 55b82a0ff..23551eda7 100644 --- a/crates/validator/src/error.rs +++ b/crates/validator/src/error.rs @@ -62,6 +62,23 @@ pub enum Error { BeaconNodeStatus { status: u16, body: String }, #[error("beacon node is syncing")] BeaconNodeSyncing, + /// A signed blinded block was sent and the node's answer does not say + /// whether it went out: a 5xx, or no answer at all. + /// + /// Distinct from [`Self::BeaconNodeStatus`], which for this call means a + /// definite refusal (a 4xx: the block was not published). Here the node + /// may have handed the block to its builder before failing, and the relay + /// publishes on receipt, so the block may well be on the network. Nothing + /// can safely be retried: the slot is signed, and a second block for it + /// would be an equivocation. `status` is `None` for a transport failure. + /// + /// Classified retryable like the other beacon-node errors, but that governs + /// only whether the next duty is attempted, not this block: it is never + /// sent again. + #[error( + "the beacon node did not confirm the blinded block (it may still be published): {detail}" + )] + BlindedUnconfirmed { status: Option, detail: String }, /// The slot's attestation work ran past the end of the slot and was /// abandoned. /// @@ -125,6 +142,7 @@ impl Error { | Self::AllBeaconNodesFailed(_) | Self::BeaconNodeStatus { .. } | Self::BeaconNodeSyncing + | Self::BlindedUnconfirmed { .. } | Self::InconsistentResponse(_) | Self::AttestationDeadline { .. } | Self::Decode(_) => true, diff --git a/crates/validator/src/lib.rs b/crates/validator/src/lib.rs index ebec73b1f..7e2732809 100644 --- a/crates/validator/src/lib.rs +++ b/crates/validator/src/lib.rs @@ -43,6 +43,7 @@ pub mod payload_attestation; pub mod proposal; pub mod proposal_guard; pub mod proposer_settings; +pub mod registration; pub(crate) mod secure_fs; pub mod signing; pub mod slot_clock; @@ -61,15 +62,16 @@ use tracing::{error, info, warn}; use crate::aggregation::AggregationService; use crate::attestation::AttestationService; -use crate::beacon_node::BeaconNodeApi; use crate::beacon_node::dto::ProposerPreparationDto; use crate::beacon_node::fallback::FallbackBeaconNode; use crate::beacon_node::http::HttpBeaconNode; +use crate::beacon_node::{BeaconNodeApi, Published}; use crate::duties::DutiesService; use crate::keys::ValidatorStore; use crate::payload_attestation::PayloadAttestationService; use crate::proposal::ProposalService; use crate::proposer_settings::ProposerSettings; +use crate::registration::RegistrationService; use crate::signing::SigningContext; use crate::slot_clock::SlotClock; use crate::sync_committee::SyncCommitteeService; @@ -95,6 +97,18 @@ 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, + /// Builder proposals, or `None` when `--builder-proposals` is off. + pub builder: Option, +} + +/// Builder proposals: register each validator with the beacon node's builder +/// 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. + pub boost_factor: u64, } /// Run the validator client until the process is stopped. @@ -202,7 +216,24 @@ 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()); - let proposal = ProposalService::new(beacon_node.clone(), context.clone(), settings.clone()); + let proposal = ProposalService::new( + beacon_node.clone(), + context.clone(), + settings.clone(), + config.builder.map(|builder| builder.boost_factor), + ) + .with_clock(clock.clone()); + let registration = config.builder.map(|builder| { + info!( + boost_factor = builder.boost_factor, + "Builder proposals on: registering validators and accepting blinded blocks" + ); + Arc::new(RegistrationService::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)?; @@ -276,7 +307,19 @@ pub async fn run(config: ValidatorConfig) -> Result<()> { &context, ); match refreshed.await { - Ok(()) => last_epoch = Some(epoch), + Ok(resolved) => { + last_epoch = Some(epoch); + // Only after a refresh that got all the way through, so a + // failing one does not respawn a full send every slot, and + // spawned so a slow or hung node never delays this epoch's + // duties: a missed registration costs at most a builder + // block, never a duty. + if let Some(registration) = ®istration { + let registration = registration.clone(); + let store = store.clone(); + tokio::spawn(registration.register_all(epoch, resolved, store)); + } + } // A beacon node that is down, syncing or answering badly will // likely answer next epoch, so keep the schedule already held // and try again. Anything else will not fix itself, and looping @@ -331,6 +374,13 @@ pub async fn run(config: ValidatorConfig) -> Result<()> { &store, ) .await; + + // Off the slot loop, like the epoch's full send: a keymanager change + // to a fee recipient or gas limit reaches the builder within a slot, + // and a slow node cannot delay the next slot's duties. + if let Some(registration) = ®istration { + tokio::spawn(registration.clone().register_changed(epoch, store.clone())); + } } } @@ -626,7 +676,7 @@ async fn aggregate( /// Failures are logged and counted rather than propagated. A proposal is one /// slot's work; losing it must not stop the client attesting for the rest of /// the epoch. -async fn propose( +async fn propose( proposal: &ProposalService, clock: &SlotClock, slot: u64, @@ -636,16 +686,37 @@ async fn propose( let budget = clock.until_attestation(slot, SystemTime::now()); let attempt = tokio::time::timeout(budget, proposal.propose(slot, duty, store)); match attempt.await { + // Already logged at error and counted on its own series; the block + // may well be out, so it is neither a published-in-time delay nor a + // failure. + Ok(Ok(Published::BuilderUnconfirmed)) => {} + // A blinded publish observes its own delay, on the task that ran it. + Ok(Ok(_)) if proposal.signed_blinded_at(slot) => {} Ok(Ok(_)) => { let elapsed = SystemTime::now() .duration_since(clock.start_of(slot)) .unwrap_or_default(); metrics::observe_block_publication_delay(elapsed.as_secs_f64()); } + // Once a blinded block is signed, only its publish task reports an + // error (and counts the failure), so it is not counted twice here. + Ok(Err(_)) if proposal.signed_blinded_at(slot) => {} Ok(Err(err)) => { error!(%slot, validator = duty.validator_index, %err, "Failed to propose this slot's block"); metrics::inc_block_proposal_failures(); } + // A blinded block is signed, and its publish runs on its own task: the + // beacon node waits up to 5 s for the relay, which can outlast this + // budget. That is not a missed proposal, and the task counts the real + // outcome. + Err(_) if proposal.signed_blinded_at(slot) => { + info!( + %slot, + validator = duty.validator_index, + budget_ms = budget.as_millis() as u64, + "Blinded block signed; its publish outlived the proposal budget and continues" + ); + } Err(_) => { warn!( %slot, @@ -660,6 +731,9 @@ async fn propose( /// Resolve any newly activated validators, refresh the duty schedule, and /// re-send subnet subscriptions if it moved. +/// +/// Returns the pubkeys the beacon node resolved to an index, which is who a +/// builder registration is signed for: the node drops the rest anyway. async fn refresh_epoch( beacon_node: &Arc, duties: &mut DutiesService, @@ -668,12 +742,12 @@ async fn refresh_epoch( settings: &ProposerSettings, store: &RwLock, context: &SigningContext, -) -> Result<()> { +) -> Result> { // Nothing to resolve, nothing to schedule, nothing to register. Returning // here also keeps an empty pubkey list away from `validator_indices`, whose // endpoint reads one as "every validator on the chain". if pubkeys.is_empty() { - return Ok(()); + return Ok(Vec::new()); } if beacon_node.is_optimistic_or_syncing().await? { @@ -744,7 +818,7 @@ async fn refresh_epoch( subscriptions::subscribe(beacon_node, &duties.all(), store, context).await?; } metrics::set_beacon_node_available(true); - Ok(()) + Ok(entries.iter().map(|entry| entry.pubkey).collect()) } /// Fetch the payload timeliness committee schedule for this epoch and the next, diff --git a/crates/validator/src/metrics.rs b/crates/validator/src/metrics.rs index 2da746bb3..64d7ca0cc 100644 --- a/crates/validator/src/metrics.rs +++ b/crates/validator/src/metrics.rs @@ -113,6 +113,46 @@ static BLOCKS_BROADCAST_NOT_IMPORTED_TOTAL: LazyLock = LazyLock::new .unwrap() }); +/// Builder registrations a beacon node accepted, counted per registration so a +/// full `REGISTRATION_BATCH` reads as that many. +static BUILDER_REGISTRATIONS_SENT_TOTAL: LazyLock = LazyLock::new(|| { + register_int_counter!( + "ethlambda_validator_builder_registrations_sent_total", + "Builder registrations a beacon node accepted" + ) + .unwrap() +}); + +/// Registration batches no beacon node accepted. A missed registration costs +/// at most a builder block, never a duty, so this is a warning sign rather +/// than a failure. +static BUILDER_REGISTRATION_FAILURES_TOTAL: LazyLock = LazyLock::new(|| { + register_int_counter!( + "ethlambda_validator_builder_registration_failures_total", + "Registration 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!( + "ethlambda_validator_blinded_blocks_proposed_total", + "Blinded blocks a beacon node handed to its builder" + ) + .unwrap() +}); + +/// Blinded blocks the beacon node did not confirm. The relay may still have +/// published them, so this is not a count of missed proposals. +static BLINDED_BLOCKS_UNCONFIRMED_TOTAL: LazyLock = LazyLock::new(|| { + register_int_counter!( + "ethlambda_validator_blinded_blocks_unconfirmed_total", + "Blinded blocks the beacon node did not confirm, which the relay may still have published" + ) + .unwrap() +}); + /// Blocks this process declined to sign because it had already proposed that /// slot for that validator in this run. /// @@ -316,6 +356,10 @@ pub fn init() { LazyLock::force(&ATTESTATION_DEADLINE_MISSED_TOTAL); LazyLock::force(&BLOCKS_PROPOSED_TOTAL); LazyLock::force(&BLOCKS_BROADCAST_NOT_IMPORTED_TOTAL); + LazyLock::force(&BUILDER_REGISTRATIONS_SENT_TOTAL); + LazyLock::force(&BUILDER_REGISTRATION_FAILURES_TOTAL); + LazyLock::force(&BLINDED_BLOCKS_PROPOSED_TOTAL); + LazyLock::force(&BLINDED_BLOCKS_UNCONFIRMED_TOTAL); LazyLock::force(&BLOCKS_REFUSED_TOTAL); LazyLock::force(&BLOCK_PROPOSAL_FAILURES_TOTAL); LazyLock::force(&ENVELOPES_PUBLISHED_TOTAL); @@ -376,6 +420,22 @@ pub fn inc_blocks_broadcast_not_imported() { BLOCKS_BROADCAST_NOT_IMPORTED_TOTAL.inc(); } +pub fn inc_builder_registrations_sent(count: usize) { + BUILDER_REGISTRATIONS_SENT_TOTAL.inc_by(count as u64); +} + +pub fn inc_builder_registration_failures() { + BUILDER_REGISTRATION_FAILURES_TOTAL.inc(); +} + +pub fn inc_blinded_blocks_proposed() { + BLINDED_BLOCKS_PROPOSED_TOTAL.inc(); +} + +pub fn inc_blinded_blocks_unconfirmed() { + BLINDED_BLOCKS_UNCONFIRMED_TOTAL.inc(); +} + pub fn inc_blocks_refused() { BLOCKS_REFUSED_TOTAL.inc(); } diff --git a/crates/validator/src/proposal.rs b/crates/validator/src/proposal.rs index 1c76e4b37..381d25a59 100644 --- a/crates/validator/src/proposal.rs +++ b/crates/validator/src/proposal.rs @@ -39,6 +39,28 @@ //! //! The envelope is not slashable, so the guard is untouched by it. //! +//! # Blinded blocks +//! +//! With builder proposals on, the node may answer with a blinded block: the +//! payload's header in place of the payload. Its root is the full block's, so +//! the guard, the signing root and the signature are exactly what a full block +//! gets, and the same slot-level protections apply. +//! +//! The fee recipient is not compared for a blinded block. The header names the +//! builder's address, and the relay pays the validator inside the payload, so +//! a mismatch there is the flow working rather than a node paying someone else. +//! +//! The signed block goes to `blinded_blocks` on the node that produced it and +//! on no other: only that node's builder holds the payload. A publish the node +//! fails to confirm is reported as [`Published::BuilderUnconfirmed`] rather +//! than as a missed proposal, since the relay may still publish the block. +//! +//! The publish runs on its own task. The beacon node waits up to 5 s for the +//! relay, which can outlast the proposal's budget (see `lib.rs`), and dropping +//! the proposal future must neither cancel the request nor lose its outcome: the +//! task logs and counts it itself, and [`ProposalService::signed_blinded_at`] +//! lets the caller tell an overrun after signing from a block never signed. +//! //! # Do not retry this call within a slot //! //! For the same reason [`crate::attestation::AttestationService::attest`] must @@ -49,6 +71,7 @@ //! dangerous, but the caller should not be relying on the guard for that. use std::sync::Arc; +use std::time::SystemTime; use ethlambda_types::beacon::constants::BUILDER_INDEX_SELF_BUILD; use ethlambda_types::beacon::fork::ForkName; @@ -65,6 +88,7 @@ use crate::keys::ValidatorStore; use crate::proposal_guard::ProposalGuard; use crate::proposer_settings::ProposerSettings; use crate::signing::SigningContext; +use crate::slot_clock::SlotClock; pub struct ProposalService { beacon_node: Arc, @@ -84,22 +108,64 @@ pub struct ProposalService { /// /// Not slashing protection. See [`ProposalGuard`]. guard: std::sync::Mutex, + /// 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 slot of the latest blinded block this service signed, set before + /// its publish starts. A `std::sync::Mutex` for the reason `guard` is one. + last_blinded_slot: std::sync::Mutex>, + /// Where a slot starts, so a blinded publish that outlives the proposal + /// can observe its own publication delay. `None` observes nothing. + clock: Option, } -impl ProposalService { +impl ProposalService { pub fn new( beacon_node: Arc, context: Arc, settings: Arc, + builder_boost_factor: Option, ) -> Self { Self { beacon_node, context, settings, + builder_boost_factor, + last_blinded_slot: std::sync::Mutex::new(None), + clock: None, guard: std::sync::Mutex::new(ProposalGuard::new()), } } + /// 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`]). + pub fn with_clock(mut self, clock: SlotClock) -> Self { + self.clock = Some(clock); + self + } + + /// Whether this service signed a blinded block for `slot`, whose publish + /// may therefore still be in flight on its own task. + pub fn signed_blinded_at(&self, slot: Slot) -> bool { + *self + .last_blinded_slot + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + == Some(slot) + } + + /// Whether the produced block's fee recipient is this client's to compare. + /// + /// Not for a blinded block: its header names the builder's address, and the + /// relay pays the validator inside the payload it reveals, so a difference + /// there is the flow working, not a node paying someone else. + fn compares_fee_recipient( + produced: &crate::beacon_node::block_contents::ProducedBlock, + ) -> bool { + !produced.is_blinded() + } + /// Compare the produced block's fee recipient against what was asked for, /// and complain loudly if they differ. /// @@ -215,6 +281,7 @@ impl ProposalService { proposer_index: duty.validator_index, randao_reveal, graffiti, + builder_boost_factor: self.builder_boost_factor, }; let mut produced = self.beacon_node.produce_block(&request).await?; @@ -252,13 +319,23 @@ impl ProposalService { ))); } - Self::check_fee_recipient(&produced, fee_recipient, slot); + if Self::compares_fee_recipient(&produced) { + Self::check_fee_recipient(&produced, fee_recipient, slot); + } else { + tracing::debug!( + %slot, + "Blinded block: its header names the builder's address; the relay pays the \ + validator in the payload" + ); + } let fork = produced.fork; + let blinded = produced.is_blinded(); info!( %slot, validator = duty.validator_index, fork = fork.as_str(), + blinded, blobs = produced.blob_count(), "Block produced; signing" ); @@ -292,7 +369,16 @@ impl ProposalService { } } - self.context.sign_block(&store, &pubkey, block_root, slot)? + let signature = self.context.sign_block(&store, &pubkey, block_root, slot)?; + // Before the first await after signing, so an overrun of the + // proposal's budget from here on is known to be a signed block. + if blinded { + *self + .last_blinded_slot + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) = Some(slot); + } + signature }; // Taken out before the block is signed into its body: the envelope is @@ -302,10 +388,18 @@ impl ProposalService { let builder_index = produced.builder_index(); let body = produced.into_signed_ssz(signature); - let published = self - .publish(fork, &body, slot, duty.validator_index) - .await?; - crate::metrics::inc_blocks_proposed(); + // A blinded block counts itself on its own task (see + // `publish_blinded`); a full one is counted here. + let published = if blinded { + self.publish_blinded(slot, fork, body, duty.validator_index) + .await? + } else { + let published = self + .publish(fork, &body, slot, duty.validator_index) + .await?; + crate::metrics::inc_blocks_proposed(); + published + }; // Only a self-built payload is this client's to reveal. if builder_index == Some(BUILDER_INDEX_SELF_BUILD) { @@ -400,6 +494,105 @@ impl ProposalService { Ok(()) } + /// Send a signed blinded block to the node that produced it. + /// + /// # On its own task + /// + /// The request, its logging and its counters run on a spawned task, so a + /// proposal future dropped at the duty loop's budget (the node may wait up + /// to 5 s on the relay) neither cancels the request nor loses its outcome. + /// This method only awaits the task. + /// + /// # Why an unconfirmed publish is not an error + /// + /// A 5xx or a missing answer does not say the block was not published: the + /// node may have handed it to the relay, which publishes regardless of what + /// the node then replied. So that outcome is logged loudly and counted on + /// its own series, and returned as [`Published::BuilderUnconfirmed`]. It is + /// not retried: the slot is signed, and a second block for it would be an + /// equivocation. A 4xx is a definite refusal and propagates as a failed + /// proposal. + async fn publish_blinded( + &self, + slot: Slot, + fork: ForkName, + body: Vec, + validator: u64, + ) -> Result { + let beacon_node = self.beacon_node.clone(); + let slot_start = self.clock.as_ref().map(|clock| clock.start_of(slot)); + let task = tokio::spawn(async move { + let bytes = body.len(); + match beacon_node.publish_blinded_block(slot, fork, &body).await { + Ok(published) => { + match published { + Published::HandedToBuilder => { + info!(%slot, validator, bytes, "Blinded block handed to the builder"); + } + Published::BroadcastNotImported => { + warn!( + %slot, + validator, + "Block was broadcast but the beacon node could not import it; \ + check that node's execution layer" + ); + crate::metrics::inc_blocks_broadcast_not_imported(); + } + // Neither belongs to this path: a full block's outcome, + // and one only this service builds. Reported rather + // than assumed away. + Published::Imported | Published::BuilderUnconfirmed => warn!( + %slot, + validator, + ?published, + "Beacon node API answered a blinded publish with an outcome that \ + belongs to another call" + ), + } + if published != Published::BuilderUnconfirmed { + crate::metrics::inc_blocks_proposed(); + crate::metrics::inc_blinded_blocks_proposed(); + // Here rather than in the duty loop, which has stopped + // waiting if the relay was slow. + if let Some(start) = slot_start { + let elapsed = + SystemTime::now().duration_since(start).unwrap_or_default(); + crate::metrics::observe_block_publication_delay(elapsed.as_secs_f64()); + } + } + Ok(published) + } + Err(err @ Error::BlindedUnconfirmed { .. }) => { + tracing::error!( + %slot, + validator, + %err, + "The beacon node did not confirm the blinded block; the relay may still \ + publish it. Not retried: the slot is signed, and a second block would be \ + an equivocation" + ); + crate::metrics::inc_blinded_blocks_unconfirmed(); + Ok(Published::BuilderUnconfirmed) + } + // A definite refusal. Reported here and not left to the caller, + // which may have stopped waiting at the proposal's budget: this + // task is the only one that knows the slot was missed. + Err(err) => { + tracing::error!(%slot, validator, %err, "The beacon node refused the blinded block"); + crate::metrics::inc_block_proposal_failures(); + Err(err) + } + } + }); + // A task that panicked has no outcome to report; the block may or may + // not have gone out, which is what "unconfirmed" says. + task.await.unwrap_or_else(|err| { + tracing::error!(%slot, validator, %err, "The blinded publish task failed"); + crate::metrics::inc_blinded_blocks_unconfirmed(); + Ok(Published::BuilderUnconfirmed) + }) + } + /// Send the signed body and report what the node made of it. /// /// Split out so the 202 case has one place to be explained rather than @@ -431,6 +624,10 @@ impl ProposalService { ); crate::metrics::inc_blocks_broadcast_not_imported(); } + // `publish_block` never answers with these: a blinded block goes + // through `publish_blinded_block`, and `BuilderUnconfirmed` is + // built by this service, not by an implementation. + Published::HandedToBuilder | Published::BuilderUnconfirmed => {} } Ok(published) } @@ -477,7 +674,7 @@ mod tests { fn service(node: Arc) -> ProposalService { let settings = ProposerSettings::new(Bytes32::repeat_byte(0xab), None); - ProposalService::new(node, context(), Arc::new(settings)) + ProposalService::new(node, context(), Arc::new(settings), None) } fn service_expecting( @@ -485,7 +682,7 @@ mod tests { fee_recipient: ExecutionAddress, ) -> ProposalService { let settings = ProposerSettings::new(Bytes32::repeat_byte(0xab), Some(fee_recipient)); - ProposalService::new(node, context(), Arc::new(settings)) + ProposalService::new(node, context(), Arc::new(settings), None) } /// A slot inside mainnet's electra era. @@ -582,7 +779,7 @@ mod tests { let (store, pubkey) = store(); let node = Arc::new(MockBeaconNode::new().with_block(slot(), 7)); let settings = Arc::new(ProposerSettings::new(Bytes32::repeat_byte(0xab), None)); - let service = ProposalService::new(node.clone(), context(), settings.clone()); + let service = ProposalService::new(node.clone(), context(), settings.clone(), None); settings.set_graffiti(&pubkey, Bytes32::repeat_byte(0xcd)); service @@ -670,6 +867,178 @@ mod tests { assert_eq!(published, Published::BroadcastNotImported); } + fn builder_service( + node: Arc, + fee_recipient: Option, + ) -> ProposalService { + let settings = ProposerSettings::new(Bytes32::repeat_byte(0xab), fee_recipient); + ProposalService::new(node, context(), Arc::new(settings), Some(100)) + } + + /// A blinded block takes the `blinded_blocks` route and nothing else. Its + /// header names the builder's address, so a configured fee recipient that + /// differs is not a mismatch for it. The mismatch counter is process-global + /// and shared with the test above, so it cannot be asserted on here; what + /// is asserted is that the proposal goes through unchanged. + #[tokio::test] + async fn a_blinded_block_is_published_to_the_builder_route() { + let (store, pubkey) = store(); + let node = Arc::new( + MockBeaconNode::new() + .with_block(slot(), 7) + .with_blinded_block(), + ); + + let service = builder_service(node.clone(), Some(H160([0xfe; 20]))); + assert!(!service.signed_blinded_at(slot())); + let published = service + .propose(slot(), &duty(&pubkey, 7, slot()), &store) + .await + .expect("proposes"); + assert_eq!(published, Published::HandedToBuilder); + assert!( + service.signed_blinded_at(slot()), + "the duty loop reads this to tell an overrun after signing from one before" + ); + assert!(!service.signed_blinded_at(slot() + 1)); + + assert_eq!(node.block_requests()[0].builder_boost_factor, Some(100)); + assert!(node.published_blocks().is_empty()); + let sent = node.published_blinded.lock().expect("lock").clone(); + assert_eq!(sent.len(), 1); + assert_eq!((sent[0].0, sent[0].1), (slot(), ForkName::Electra)); + } + + /// The fee-recipient comparison is for a full block only: a blinded + /// block's header names the builder's address. + #[tokio::test] + async fn only_a_full_block_has_its_fee_recipient_compared() { + use crate::beacon_node::block_contents::{Contents, ProducedBlock, empty_block_for}; + use ethlambda_types::beacon::builder::BlindedBeaconBlock; + + let blinded = ProducedBlock { + fork: ForkName::Electra, + contents: Contents::Blinded { + block: BlindedBeaconBlock::from(empty_block_for(slot(), 7)), + }, + }; + assert!(!ProposalService::::compares_fee_recipient( + &blinded + )); + + let full = MockBeaconNode::new().with_block(slot(), 7); + let request = BlockRequest { + slot: slot(), + fork: ForkName::Electra, + proposer_index: 7, + randao_reveal: Default::default(), + graffiti: Default::default(), + builder_boost_factor: None, + }; + let full = full.produce_block(&request).await.expect("produces"); + assert!(ProposalService::::compares_fee_recipient( + &full + )); + } + + /// 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. + #[tokio::test] + async fn an_unconfirmed_blinded_publish_is_reported_and_the_slot_is_spent() { + let (store, pubkey) = store(); + let node = Arc::new( + MockBeaconNode::new() + .with_block(slot(), 7) + .with_blinded_block() + .with_unconfirmed_blinded_publish("relay timed out"), + ); + let service = builder_service(node.clone(), None); + + let published = service + .propose(slot(), &duty(&pubkey, 7, slot()), &store) + .await + .expect("an unconfirmed publish is not an error"); + assert_eq!(published, Published::BuilderUnconfirmed); + + let err = service + .propose(slot(), &duty(&pubkey, 7, slot()), &store) + .await + .expect_err("the guard must refuse a second block for the slot"); + assert!(matches!(err, Error::ProposalRefused { .. }), "got {err:?}"); + } + + /// The duty loop drops the proposal at its budget while the node is still + /// waiting on the relay. The publish runs on its own task, so the block is + /// still sent, and the slot is known to be signed. + #[tokio::test] + async fn a_blinded_publish_survives_a_dropped_proposal() { + let (store, pubkey) = store(); + let node = Arc::new( + MockBeaconNode::new() + .with_block(slot(), 7) + .with_blinded_block() + .with_blinded_publish_delay(std::time::Duration::from_millis(300)), + ); + let service = builder_service(node.clone(), None); + + let outcome = tokio::time::timeout( + std::time::Duration::from_millis(50), + service.propose(slot(), &duty(&pubkey, 7, slot()), &store), + ) + .await; + assert!( + outcome.is_err(), + "the proposal must be dropped at the budget" + ); + assert!(service.signed_blinded_at(slot())); + assert!(node.published_blinded.lock().expect("lock").is_empty()); + + tokio::time::sleep(std::time::Duration::from_millis(600)).await; + assert_eq!( + node.published_blinded.lock().expect("lock").len(), + 1, + "the publish must complete although the proposal future is gone" + ); + } + + /// A 4xx is a definite refusal: the block was not published, so the + /// proposal failed. + #[tokio::test] + async fn a_definitely_refused_blinded_publish_is_an_error() { + let (store, pubkey) = store(); + let node = MockBeaconNode::new() + .with_block(slot(), 7) + .with_blinded_block() + .with_refused_blinded_publish(400); + + let result = builder_service(Arc::new(node), None) + .propose(slot(), &duty(&pubkey, 7, slot()), &store) + .await; + assert!( + matches!(result, Err(Error::BeaconNodeStatus { status: 400, .. })), + "got {result:?}" + ); + } + + #[tokio::test] + async fn without_builder_proposals_the_request_carries_no_factor() { + let (store, pubkey) = store(); + let node = Arc::new( + MockBeaconNode::new() + .with_block(slot(), 7) + .with_blinded_block(), + ); + + let published = service(node.clone()) + .propose(slot(), &duty(&pubkey, 7, slot()), &store) + .await + .expect("proposes"); + assert_eq!(published, Published::Imported); + assert_eq!(node.block_requests()[0].builder_boost_factor, None); + assert!(node.published_blinded.lock().expect("lock").is_empty()); + } + /// A duty naming a key this client does not hold must fail before anything /// is asked of the beacon node. #[tokio::test] @@ -827,7 +1196,7 @@ mod gloas_tests { ethlambda_types::beacon::primitives::Bytes32::repeat_byte(0xab), None, ); - ProposalService::new(node, context(), Arc::new(settings)) + ProposalService::new(node, context(), Arc::new(settings), None) } fn verifies(pubkey: &BlsPubkey, signature: &[u8; 96], root: Root) -> bool { @@ -904,7 +1273,7 @@ mod gloas_tests { )); let settings = Arc::new(ProposerSettings::new(Bytes32::repeat_byte(0xab), None)); settings.set_graffiti(&pubkey, Bytes32::repeat_byte(0xcd)); - ProposalService::new(node.clone(), context(), settings) + ProposalService::new(node.clone(), context(), settings, None) .propose(slot(), &duty(&pubkey), &store) .await .expect("proposes"); diff --git a/crates/validator/src/proposer_settings.rs b/crates/validator/src/proposer_settings.rs index 01daf3f4d..5000779c9 100644 --- a/crates/validator/src/proposer_settings.rs +++ b/crates/validator/src/proposer_settings.rs @@ -22,6 +22,7 @@ //! signing reports as a mismatch. use std::collections::HashMap; +use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::{PoisonError, RwLock}; use ethlambda_types::beacon::primitives::{BlsPubkey, Bytes32, ExecutionAddress, H256}; @@ -59,9 +60,9 @@ pub fn graffiti_to_text(graffiti: &Bytes32) -> String { /// The gas limit a validator with no override reports. /// -/// Lighthouse's and Nimbus's default. Nothing in this client uses it yet: it -/// matters only to builder registrations, and there is no builder flow. It is -/// held so the keymanager API can serve it as the specification requires. +/// 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. pub const DEFAULT_GAS_LIMIT: u64 = 60_000_000; /// One validator's overrides; `None` means the default applies. @@ -98,6 +99,10 @@ struct Inner { #[derive(Debug)] pub struct ProposerSettings { inner: RwLock, + /// Bumped on every override write or delete, so a reader that caches + /// something derived from the settings (the builder registrations) can + /// tell that nothing changed without reading every key. + generation: AtomicU64, } impl ProposerSettings { @@ -111,9 +116,16 @@ impl ProposerSettings { default_gas_limit: DEFAULT_GAS_LIMIT, overrides: HashMap::new(), }), + generation: AtomicU64::new(0), } } + /// 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 { + self.generation.load(Ordering::Acquire) + } + fn read(&self, f: impl FnOnce(&Inner) -> T) -> T { f(&self.inner.read().unwrap_or_else(PoisonError::into_inner)) } @@ -127,6 +139,9 @@ impl ProposerSettings { if entry.is_empty() { inner.overrides.remove(pubkey); } + // Inside the write lock, so a reader that sees the new generation also + // sees the new value. + self.generation.fetch_add(1, Ordering::Release); } /// Where `pubkey`'s execution-layer rewards should go, or `None` when @@ -183,6 +198,7 @@ impl ProposerSettings { pub fn forget(&self, pubkey: &BlsPubkey) { let mut inner = self.inner.write().unwrap_or_else(PoisonError::into_inner); inner.overrides.remove(pubkey); + self.generation.fetch_add(1, Ordering::Release); } } diff --git a/crates/validator/src/registration.rs b/crates/validator/src/registration.rs new file mode 100644 index 000000000..c9d5a717d --- /dev/null +++ b/crates/validator/src/registration.rs @@ -0,0 +1,571 @@ +//! Builder registrations: what tells a beacon node's builder where to pay each +//! validator and which gas limit to target. +//! +//! A registration is a `ValidatorRegistrationV1` the validator signs, so a +//! relay can trust that the fee recipient and gas limit it builds for were +//! chosen by the key that will sign the block. The beacon node forwards them to +//! its builder, which keeps the newest per validator. +//! +//! # Why a signature is reused until the settings change +//! +//! Builders order a validator's registrations by `timestamp` and keep the +//! latest, so re-sending an unchanged registration with the same timestamp is +//! harmless, and re-signing it with a fresh one every epoch would cost a BLS +//! signature per validator for a message that says nothing new. The signed +//! registration is therefore cached per validator and re-signed, with a new +//! timestamp, only when the fee recipient or gas limit changes. The full set is +//! still re-sent every epoch: a beacon node forgets its registrations when it +//! restarts, and a relay may expire them, so repeating them is what keeps the +//! validators registered without any state beyond this cache. +//! +//! # Why it stops at gloas +//! +//! From gloas the builder market works through proposer preferences, which the +//! beacon node serves itself, so there is nothing for this client to register. +//! Both entry points return at the gloas fork epoch. +//! +//! # Why every node gets them +//! +//! A registration installs state on a node's builder, so the fallback client +//! sends to every node rather than the first that answers: a node that never +//! heard them cannot serve a bid for these validators after a failover. +//! +//! # Failures are never propagated +//! +//! A missed registration costs at most a builder block, never a duty, so a +//! failed send is logged and counted and the caller moves on. A changed +//! registration whose send fails is cached anyway, so it is not re-sent at the +//! next slot: it waits for the next epoch's full send. +//! +//! # Signing stays off the async runtime +//! +//! The first epoch signs one BLS signature per validator. That runs on the +//! blocking pool in chunks of [`SIGN_CHUNK`], taking the store's read lock +//! afresh for each chunk, so neither the runtime's threads nor a queued +//! keymanager writer wait on thousands of signatures. +//! +//! # The per-slot check is one atomic read +//! +//! Only a keymanager write changes what a registration says, so +//! [`RegistrationService::register_changed`] compares +//! [`ProposerSettings::generation`] with the one it last signed against and +//! returns before touching the store when it has not moved. + +use std::collections::{HashMap, HashSet}; +use std::sync::Arc; +use std::sync::atomic::{AtomicU64, Ordering}; +use std::time::{SystemTime, UNIX_EPOCH}; + +use ethlambda_types::beacon::builder::{SignedValidatorRegistrationV1, ValidatorRegistrationV1}; +use ethlambda_types::beacon::fork::ForkName; +use ethlambda_types::beacon::primitives::{BlsPubkey, Epoch}; +use tokio::sync::RwLock; +use tracing::{debug, warn}; + +use crate::beacon_node::BeaconNodeApi; +use crate::beacon_node::dto::encode_hex; +use crate::keys::ValidatorStore; +use crate::proposer_settings::ProposerSettings; +use crate::signing::SigningContext; + +/// Registrations per request. Lighthouse's batch size: large enough that a few +/// thousand validators take a handful of requests, small enough that one +/// request stays well under a node's body limit. +pub const REGISTRATION_BATCH: usize = 500; + +/// Keys signed per blocking task, and so per hold of the store's read lock. +const SIGN_CHUNK: usize = 64; + +/// What [`RegistrationService::current`] found for a set of validators. +struct Current { + /// Every validator's registration, cached or newly signed. + all: Vec, + /// The subset signed in this call, because it had no registration yet or + /// its fee recipient or gas limit changed. + changed: Vec, + /// Validators skipped because no fee recipient is configured for them. + no_fee_recipient: usize, +} + +impl Current { + fn extend(&mut self, other: Current) { + self.all.extend(other.all); + self.changed.extend(other.changed); + self.no_fee_recipient += other.no_fee_recipient; + } +} + +/// Signs and sends builder registrations. +pub struct RegistrationService { + beacon_node: Arc, + context: Arc, + settings: Arc, + /// The validators the beacon node resolved at the last epoch refresh. + known: std::sync::Mutex>, + /// The latest signed registration per validator. + signed: std::sync::Mutex>, + /// The [`ProposerSettings::generation`] the cache was last brought up to + /// date against. + signed_generation: AtomicU64, +} + +impl RegistrationService { + pub fn new( + beacon_node: Arc, + context: Arc, + settings: Arc, + ) -> Self { + Self { + beacon_node, + context, + settings, + known: std::sync::Mutex::new(Vec::new()), + signed: std::sync::Mutex::new(HashMap::new()), + signed_generation: AtomicU64::new(0), + } + } + + /// The registration for each of `pubkeys` as of `now_secs`, signing only + /// the ones whose `(fee_recipient, gas_limit)` differs from the cached one. + /// + /// `now_secs` is a parameter so a test can fix the timestamp. + fn current(&self, store: &ValidatorStore, pubkeys: &[BlsPubkey], now_secs: u64) -> Current { + // A poisoned lock is recovered: every write is one map operation, so + // the data behind it is never half-changed. + let mut signed = self + .signed + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let mut current = Current { + all: Vec::with_capacity(pubkeys.len()), + changed: Vec::new(), + no_fee_recipient: 0, + }; + for pubkey in pubkeys { + let Some(fee_recipient) = self.settings.fee_recipient(pubkey) else { + current.no_fee_recipient += 1; + continue; + }; + let gas_limit = self.settings.gas_limit(pubkey); + + if let Some(held) = signed.get(pubkey) + && held.message.fee_recipient == fee_recipient + && held.message.gas_limit == gas_limit + { + current.all.push(held.clone()); + continue; + } + + let message = ValidatorRegistrationV1 { + fee_recipient, + gas_limit, + timestamp: now_secs, + pubkey: *pubkey, + }; + // A key the keymanager removed since the epoch refresh is the one + // way this fails; the next refresh no longer lists it. + let signature = match self.context.sign_validator_registration(store, &message) { + Ok(signature) => signature, + Err(err) => { + warn!(pubkey = %encode_hex(&pubkey.0), %err, "Could not sign a builder registration; skipping it"); + continue; + } + }; + let registration = SignedValidatorRegistrationV1 { message, signature }; + signed.insert(*pubkey, registration.clone()); + current.all.push(registration.clone()); + current.changed.push(registration); + } + current + } + + /// Whether `epoch` is at or after gloas, where there is nothing to register. + fn is_gloas(&self, epoch: Epoch) -> bool { + self.context.config.fork_at_epoch(epoch) >= ForkName::Gloas + } + + fn now_secs() -> u64 { + SystemTime::now() + .duration_since(UNIX_EPOCH) + .map_or(0, |elapsed| elapsed.as_secs()) + } + + /// [`Self::current`] over `pubkeys`, off the async runtime, [`SIGN_CHUNK`] + /// keys per blocking task with the store's read lock taken per chunk. + async fn current_blocking( + self: &Arc, + store: &Arc>, + pubkeys: &[BlsPubkey], + ) -> Current { + let now_secs = Self::now_secs(); + let mut current = Current { + all: Vec::with_capacity(pubkeys.len()), + changed: Vec::new(), + no_fee_recipient: 0, + }; + for chunk in pubkeys.chunks(SIGN_CHUNK) { + let (service, store, chunk) = (self.clone(), store.clone(), chunk.to_vec()); + let part = tokio::task::spawn_blocking(move || { + let store = store.blocking_read(); + service.current(&store, &chunk, now_secs) + }) + .await; + match part { + Ok(part) => current.extend(part), + Err(err) => warn!(%err, "Signing builder registrations failed; skipping a chunk"), + } + } + current + } + + /// Register every validator in `pubkeys`, the ones the beacon node resolved + /// at this epoch's refresh. Called once per epoch. + /// + /// Also forgets any validator no longer in `pubkeys`, so a key the + /// keymanager removed stops being re-signed (and warned about) every slot. + pub async fn register_all( + self: Arc, + epoch: Epoch, + pubkeys: Vec, + store: Arc>, + ) { + if self.is_gloas(epoch) { + return; + } + // Read before signing, so a change that lands while this runs is seen + // by the next `register_changed` rather than lost. + let generation = self.settings.generation(); + let current_keys: HashSet = pubkeys.iter().copied().collect(); + self.signed + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .retain(|pubkey, _| current_keys.contains(pubkey)); + *self + .known + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) = pubkeys.clone(); + + let current = self.current_blocking(&store, &pubkeys).await; + self.signed_generation.store(generation, Ordering::Release); + if current.no_fee_recipient > 0 { + warn!( + skipped = current.no_fee_recipient, + "Validators without a fee recipient are not registered with the builder" + ); + } + self.send(current.all, REGISTRATION_BATCH).await; + } + + /// Send only the registrations whose fee recipient or gas limit changed + /// since they were last signed, over the validators the last + /// [`Self::register_all`] was given. Called every slot, so a keymanager + /// change reaches the builder within a slot rather than an epoch. + /// + /// Returns before touching the store when no setting was written since the + /// last signing: see the module doc. + /// + /// Silent about validators with no fee recipient: that is reported once + /// per epoch by [`Self::register_all`], not repeated every slot. + pub async fn register_changed( + self: Arc, + epoch: Epoch, + store: Arc>, + ) { + if self.is_gloas(epoch) { + return; + } + let generation = self.settings.generation(); + if generation == self.signed_generation.load(Ordering::Acquire) { + return; + } + let known = self + .known + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .clone(); + if known.is_empty() { + return; + } + let current = self.current_blocking(&store, &known).await; + self.signed_generation.store(generation, Ordering::Release); + if current.changed.is_empty() { + return; + } + self.send(current.changed, REGISTRATION_BATCH).await; + } + + /// Send `registrations` to the beacon node in chunks of `batch_size`. + /// + /// The size is a parameter so a test can see the chunking without signing + /// hundreds of keys. + async fn send(&self, registrations: Vec, batch_size: usize) { + for chunk in registrations.chunks(batch_size) { + match self.beacon_node.register_validators(chunk).await { + Ok(()) => { + debug!(count = chunk.len(), "Builder registrations sent"); + crate::metrics::inc_builder_registrations_sent(chunk.len()); + } + Err(err) => { + warn!(count = chunk.len(), %err, "No beacon node accepted the builder registrations"); + crate::metrics::inc_builder_registration_failures(); + } + } + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::beacon_node::mock::MockBeaconNode; + use ethlambda_types::beacon::config::Config; + use ethlambda_types::beacon::primitives::{Bytes32, H160, Root}; + + fn context(config: Config) -> Arc { + Arc::new(SigningContext { + config, + genesis_validators_root: Root::ZERO, + }) + } + + /// `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, + ) -> ( + Arc>, + Arc, + ) { + let node = Arc::new(MockBeaconNode::new()); + ( + Arc::new(RegistrationService::new( + node.clone(), + context(config), + settings, + )), + node, + ) + } + + #[test] + fn registrations_are_signed_once_and_reused() { + let (store, pubkeys) = store_with_keys(2); + let (service, _) = service(Config::mainnet(), settings(Some(H160([0xab; 20])))); + + let first = service.current(&store, &pubkeys, 1_000); + assert_eq!(first.all.len(), 2); + assert_eq!(first.changed.len(), 2); + + let second = service.current(&store, &pubkeys, 2_000); + assert_eq!(second.all, first.all, "same timestamp and signature"); + assert!(second.changed.is_empty()); + assert_eq!(second.all[0].message.timestamp, 1_000); + } + + #[test] + fn a_new_fee_recipient_or_gas_limit_re_signs() { + let (store, pubkeys) = store_with_keys(3); + let settings = settings(Some(H160([0xab; 20]))); + let (service, _) = service(Config::mainnet(), settings.clone()); + let first = service.current(&store, &pubkeys, 1_000); + + settings.set_fee_recipient(&pubkeys[0], H160([0xcd; 20])); + settings.set_gas_limit(&pubkeys[1], 30_000_000); + let second = service.current(&store, &pubkeys, 2_000); + + let changed: Vec<_> = second.changed.iter().map(|r| r.message.pubkey).collect(); + assert_eq!(changed, vec![pubkeys[0], pubkeys[1]]); + assert_eq!(second.all[0].message.fee_recipient, H160([0xcd; 20])); + assert_eq!(second.all[0].message.timestamp, 2_000); + assert_eq!(second.all[1].message.gas_limit, 30_000_000); + assert_eq!(second.all[1].message.timestamp, 2_000); + assert_ne!(second.all[0].signature, first.all[0].signature); + assert_eq!(second.all[2], first.all[2], "the third is untouched"); + } + + #[test] + fn a_validator_without_a_fee_recipient_is_not_registered() { + let (store, pubkeys) = store_with_keys(2); + let settings = settings(None); + settings.set_fee_recipient(&pubkeys[1], H160([0xcd; 20])); + let (service, _) = service(Config::mainnet(), settings); + + let current = service.current(&store, &pubkeys, 1_000); + + assert_eq!(current.no_fee_recipient, 1); + assert_eq!(current.all.len(), 1); + assert_eq!(current.all[0].message.pubkey, pubkeys[1]); + } + + #[test] + fn a_removed_key_is_skipped_rather_than_failing_the_rest() { + let (mut store, pubkeys) = store_with_keys(2); + let (service, _) = service(Config::mainnet(), settings(Some(H160([0xab; 20])))); + store.remove(&pubkeys[0]); + + let current = service.current(&store, &pubkeys, 1_000); + + assert_eq!(current.all.len(), 1); + assert_eq!(current.all[0].message.pubkey, pubkeys[1]); + } + + /// 501 keys is one more than a batch; signing them is a millisecond each. + #[tokio::test] + async fn register_all_sends_in_batches_of_500() { + assert_eq!(REGISTRATION_BATCH, 500); + let (store, pubkeys) = store_with_keys(REGISTRATION_BATCH + 1); + let (service, node) = service(Config::mainnet(), settings(Some(H160([0xab; 20])))); + let store = Arc::new(RwLock::new(store)); + + service.register_all(0, pubkeys, store).await; + + assert_eq!( + *node.registration_batches.lock().expect("lock"), + vec![REGISTRATION_BATCH, 1] + ); + } + + #[tokio::test] + async fn send_chunks_by_the_size_it_is_given() { + let (store, pubkeys) = store_with_keys(5); + let (service, node) = service(Config::mainnet(), settings(Some(H160([0xab; 20])))); + let current = service.current(&store, &pubkeys, 1_000); + + service.send(current.all, 2).await; + + assert_eq!( + *node.registration_batches.lock().expect("lock"), + vec![2, 2, 1] + ); + } + + /// Both entry points stop at gloas, and `register_changed` is checked with + /// something to send: a service that was filled before the fork and then + /// had a setting changed. + #[tokio::test] + async fn nothing_is_sent_at_or_after_gloas() { + let (store, pubkeys) = store_with_keys(2); + let config = Config { + gloas_fork_epoch: 4, + ..Config::mainnet() + }; + let settings = settings(Some(H160([0xab; 20]))); + let (service, node) = service(config, settings.clone()); + let store = Arc::new(RwLock::new(store)); + + // Before the fork the service registers, and holds the keys. + service + .clone() + .register_all(3, pubkeys.clone(), store.clone()) + .await; + assert_eq!(*node.registration_batches.lock().expect("lock"), vec![2]); + + settings.set_gas_limit(&pubkeys[0], 30_000_000); + service + .clone() + .register_all(4, pubkeys, store.clone()) + .await; + service.clone().register_changed(5, store.clone()).await; + assert_eq!( + *node.registration_batches.lock().expect("lock"), + vec![2], + "a changed key is still not sent at or after the fork" + ); + + // The same change is sent when the epoch is before the fork. + service.register_changed(3, store).await; + assert_eq!(*node.registration_batches.lock().expect("lock"), vec![2, 1]); + } + + #[tokio::test] + async fn register_changed_sends_only_the_changed_keys() { + let (store, pubkeys) = store_with_keys(3); + let settings = settings(Some(H160([0xab; 20]))); + let (service, node) = service(Config::mainnet(), settings.clone()); + let store = Arc::new(RwLock::new(store)); + + service + .clone() + .register_all(0, pubkeys.clone(), store.clone()) + .await; + service.clone().register_changed(0, store.clone()).await; + assert_eq!( + *node.registration_batches.lock().expect("lock"), + vec![3], + "nothing changed, so nothing more is sent" + ); + + settings.set_gas_limit(&pubkeys[2], 30_000_000); + service.register_changed(0, store).await; + + let registrations = node.registrations.lock().expect("lock"); + assert_eq!(registrations.len(), 4); + assert_eq!(registrations[3].message.pubkey, pubkeys[2]); + assert_eq!(registrations[3].message.gas_limit, 30_000_000); + } + + /// The per-slot call is one atomic read when no setting moved: with the + /// store write-locked, so any read of it would wait forever, it still + /// returns at once, and sends nothing. + #[tokio::test] + async fn a_slot_with_no_change_makes_no_store_read_and_no_send() { + let (store, pubkeys) = store_with_keys(2); + let (service, node) = service(Config::mainnet(), settings(Some(H160([0xab; 20])))); + let store = Arc::new(RwLock::new(store)); + service + .clone() + .register_all(0, pubkeys, store.clone()) + .await; + let signed_before = service.signed.lock().expect("lock").clone(); + + let _writer = store.write().await; + tokio::time::timeout( + std::time::Duration::from_millis(200), + service.clone().register_changed(0, store.clone()), + ) + .await + .expect("returns without waiting on the store"); + + assert_eq!(*node.registration_batches.lock().expect("lock"), vec![2]); + assert_eq!(*service.signed.lock().expect("lock"), signed_before); + } + + /// A key that left the resolved set is dropped from the cache, so it is not + /// re-signed (and warned about) at every change. + #[tokio::test] + async fn a_key_no_longer_resolved_is_forgotten() { + let (store, pubkeys) = store_with_keys(2); + let (service, _node) = service(Config::mainnet(), settings(Some(H160([0xab; 20])))); + let store = Arc::new(RwLock::new(store)); + + service + .clone() + .register_all(0, pubkeys.clone(), store.clone()) + .await; + service + .clone() + .register_all(0, vec![pubkeys[1]], store) + .await; + + let signed = service.signed.lock().expect("lock"); + assert_eq!(signed.keys().copied().collect::>(), vec![pubkeys[1]]); + assert_eq!(*service.known.lock().expect("lock"), vec![pubkeys[1]]); + } +} diff --git a/crates/validator/src/signing.rs b/crates/validator/src/signing.rs index ced20d40e..cafa4099a 100644 --- a/crates/validator/src/signing.rs +++ b/crates/validator/src/signing.rs @@ -5,6 +5,7 @@ //! beacon node sent: the wire representation is a transport detail, and the //! chain only ever agrees about the merkle root. +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, @@ -25,7 +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::signing::{ - compute_domain, compute_epoch_at_slot, compute_signing_root, + compute_builder_domain, compute_domain, compute_epoch_at_slot, compute_signing_root, }; use crate::error::{Error, Result}; @@ -297,6 +298,24 @@ impl SigningContext { self.sign_root(store, pubkey, self.envelope_signing_root(envelope, slot)) } + /// A builder registration's signature. + /// + /// Signed under `compute_builder_domain(genesis_fork_version)`: the genesis + /// fork version and a zero validators root, per builder-specs, so a + /// registration verifies whatever fork the chain is in. Not + /// `DOMAIN_BEACON_BUILDER`, which is gloas's envelope domain. Not + /// slashable: a registration commits to nothing about the chain, and a + /// builder keeps only the newest one per validator. + pub fn sign_validator_registration( + &self, + store: &ValidatorStore, + registration: &ValidatorRegistrationV1, + ) -> Result { + let domain = compute_builder_domain(self.config.genesis_fork_version); + let root = compute_signing_root(registration.hash_tree_root(), domain); + self.sign_root(store, ®istration.pubkey, root) + } + /// Sign a payload timeliness committee vote on behalf of `pubkey`. /// /// Not slashable by the specification, and deduplicated per validator and @@ -506,6 +525,71 @@ mod tests { (store, pubkey) } + fn registration(pubkey: BlsPubkey) -> ValidatorRegistrationV1 { + ValidatorRegistrationV1 { + fee_recipient: ethlambda_types::beacon::primitives::H160([0xab; 20]), + gas_limit: 36_000_000, + timestamp: 1_700_000_000, + pubkey, + } + } + + /// A context whose genesis validators root is not zero, so the builder + /// domain (which zeroes it) and a context-derived one cannot coincide. + fn context_with_validators_root() -> SigningContext { + SigningContext { + config: Config::mainnet(), + genesis_validators_root: Root::repeat_byte(0x42), + } + } + + #[test] + fn a_registration_is_signed_under_the_builder_domain() { + use ethlambda_types::beacon::constants::DOMAIN_APPLICATION_BUILDER; + + let (store, pubkey) = store_with_key(); + let context = context_with_validators_root(); + let registration = registration(pubkey); + + let signature = context + .sign_validator_registration(&store, ®istration) + .expect("signs"); + + let root = compute_signing_root( + registration.hash_tree_root(), + compute_builder_domain(Config::mainnet().genesis_fork_version), + ); + assert!(verify(&pubkey, &signature, root)); + + // The same domain type under this chain's validators root is a + // different domain: a context-derived one would not verify. + let with_root = compute_signing_root( + registration.hash_tree_root(), + context.domain(DOMAIN_APPLICATION_BUILDER, 0), + ); + assert!(!verify(&pubkey, &signature, with_root)); + } + + /// The registration domain is not gloas's envelope domain, whose type + /// constant is similar in name only: a signature under one must not verify + /// under the other. + #[test] + fn a_registration_does_not_verify_under_the_envelope_domain() { + let (store, pubkey) = store_with_key(); + let context = context_with_validators_root(); + let registration = registration(pubkey); + + let signature = context + .sign_validator_registration(&store, ®istration) + .expect("signs"); + + let envelope_root = compute_signing_root( + registration.hash_tree_root(), + context.domain(DOMAIN_BEACON_BUILDER, 0), + ); + assert!(!verify(&pubkey, &signature, envelope_root)); + } + #[test] fn a_randao_reveal_verifies_under_the_randao_signing_root() { let (store, pubkey) = store_with_key(); diff --git a/docs/cli.md b/docs/cli.md index b9a65aa5d..998f1e8fd 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -290,6 +290,9 @@ 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-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` | +| `--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 @@ -303,6 +306,17 @@ validator's fee recipient with the beacon node. The subscription also tells the node which committees this client will aggregate for, which it has to know in advance so it can collect the votes. +With `--builder-proposals`, before gloas, it also signs a builder registration +(fee recipient and gas limit) for each validator the node resolved and sends +them to every node: the full set after each epoch's refresh, in batches, and +every slot only the registrations whose fee recipient or gas limit changed +through the keymanager API. A block request then carries a +`builder_boost_factor`, and when the node answers with a blinded block it is +signed like a full one and published to `blinded_blocks` on the node that +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. + 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 @@ -323,7 +337,12 @@ slot. The specification defines no block-production deadline; the nearest thing it defines is the point attesters stop waiting for a block, which is the same instant. A proposal still running then has already lost most of the block's value, and every second past it comes out of this client's own attestations, so -it is abandoned and the slot's attesters still vote. +it is abandoned and the slot's attesters still vote. A blinded block that was +already signed is the exception: its publish runs on its own task, since the +beacon node may wait several seconds on the relay, so an overrun after signing +is logged at info and not counted as a failed proposal, and the task counts the +real outcome (a refusal as a failed proposal, success with its publication +delay). Electra is the earliest fork it will propose under. A deneb block body has one field fewer than an electra one, so it would need a container pair of its own, @@ -416,9 +435,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 | +| `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 | | `graffiti` | `--graffiti` | The next proposal | -| `gas_limit` | `DEFAULT_GAS_LIMIT` | Stored and served only: it matters to builder registrations, and there is no builder flow | +| `gas_limit` | `DEFAULT_GAS_LIMIT` | With `--builder-proposals`, the next slot's builder registration, which the builder builds toward. Otherwise stored and served only | **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. diff --git a/docs/metrics.md b/docs/metrics.md index 087d990f0..1b63a76be 100644 --- a/docs/metrics.md +++ b/docs/metrics.md @@ -741,6 +741,10 @@ and absent is not the same as zero. | `ethlambda_validator_aggregates_published_total` | Counter | Aggregates accepted by a beacon node. Bursty rather than steady: a validator is selected a few times a day, so hours at zero are normal for a small deployment | | `ethlambda_validator_aggregation_failures_total` | Counter | Aggregation duties that ended in no published aggregate, including ones abandoned for overrunning the slot | | `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_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 | | `ethlambda_validator_beacon_node_available` | Gauge | 1 when a beacon node answered the last duty refresh or attestation | | `ethlambda_validator_signing_duration_seconds` | Histogram | Time to sign one slot's batch, store read lock included | @@ -755,13 +759,17 @@ second points at the beacon nodes; a rise in the third points at the clock or the duty schedule and is worth investigating even though the attestation was correctly suppressed. -The proposal series split the same way, plus one that is neither a success nor +The proposal series split the same way, plus two that are neither a success nor a failure. `blocks_proposed_total` counts blocks a node accepted; `block_proposal_failures_total` counts duties that produced none; `blocks_refused_total` counts blocks deliberately not signed. Between them, `blocks_broadcast_not_imported_total` counts blocks that did reach the network but that the node answering could not import, which is a beacon-node fault rather than a validator one and is why it is not folded into either. +`blinded_blocks_unconfirmed_total` is the other: a blinded block the node did +not confirm, which the relay may still have published. It is not in +`blocks_proposed_total` nor in the publication delay histogram, and it is not a +failure either. Gloas adds two pairs. A gloas proposal ends in a block and then an envelope, and the envelope's failure does not fail the proposal: `blocks_proposed_total` diff --git a/docs/spec_deviations.md b/docs/spec_deviations.md index 8e04fb262..84a27a23d 100644 --- a/docs/spec_deviations.md +++ b/docs/spec_deviations.md @@ -683,3 +683,29 @@ comes first. The validator client signs once, at the deadline (`SYNC_MESSAGE_DUE_BPS`, or its gloas variant), over the head root it then reads, and refuses to when that root is optimistic (`specs/bellatrix/optimistic-sync.md`). + +## The validator client's builder proposals + +With `--builder-proposals` the validator client registers its validators with +the beacon node's builder and may sign a blinded block (builder-specs and +`validator.md`'s builder flow), for fulu and earlier slots. Three departures: + +- **No slashing-protection record for a blinded block either.** It is signed + without a durable record, like every other block from this client. A blinded + block has the root of the full block it stands for, so the in-process + `ProposalGuard` covers both kinds: one slot, one signature, whichever way the + node answered. +- **A blinded block is published only on the beacon node that produced it, with + no failover.** This is this client's choice, not the specification's: only + that node's builder holds the payload and only that node claimed the slot, so + offering the signed block to another node would hand it to a builder that + never saw the bid. If the producing node fails, the publish fails, and + the guard has already recorded the slot, so nothing retries. A 5xx or no + answer is reported as unconfirmed (logged at error, counted on + `blinded_blocks_unconfirmed_total`) and not as a missed proposal, since the + 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. + Validators with no fee recipient are not registered either (the field is + required), and only validators the beacon node resolved are signed for.