diff --git a/CLAUDE.md b/CLAUDE.md index 482ec53d..58798eb3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -862,11 +862,14 @@ 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 keeps EIP-3076 minimal-watermark slashing protection in `--validators-dir/slashing_protection.json` (`crate::slashing_protection`), - durable before every block and attestation signature; PTC votes have an - in-memory `(validator, slot)` dedup only, and envelopes are unguarded. + durable before every block (blinded ones included) and attestation signature; + PTC votes have an in-memory `(validator, slot)` dedup only, and envelopes are + unguarded. `get_head_node` is one bottom-up walk for every fork, with the current slot's fork selecting the payload rules and the boost gate, so a follower on any network that schedules gloas (Sepolia does) pays work diff --git a/Cargo.lock b/Cargo.lock index 405f49c0..3080e495 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2274,6 +2274,7 @@ dependencies = [ "ctr", "ethlambda-metrics", "ethlambda-types", + "futures-util", "hex", "hmac", "http-body-util", diff --git a/README.md b/README.md index 5b401a5b..986e5d14 100644 --- a/README.md +++ b/README.md @@ -369,6 +369,7 @@ the standard Beacon API, so it works against `ethlambda beacon` (run with the | `--secrets-dir` | One password file per keystore, named after the validator's public key | | `--suggested-fee-recipient` | Execution address for block rewards, `0x`-prefixed. Optional, but without it the beacon node picks an address that is not yours | | `--graffiti` | Text for proposed blocks, at most 32 bytes | +| `--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 96824496..3efbfc03 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; @@ -79,6 +79,31 @@ pub(crate) struct ValidatorOptions { #[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, + /// Register every loaded key that has no slashing-protection record, /// instead of refusing to start. /// @@ -134,6 +159,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<()> { @@ -157,6 +197,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, @@ -167,6 +208,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, init_slashing_protection: options.init_slashing_protection, }) .await @@ -188,10 +230,90 @@ mod tests { graffiti: graffiti.to_string(), suggested_fee_recipient: None, enable_keymanager: false, + builder_proposals: false, + builder_boost_factor: 100, + prefer_builder_proposals: false, init_slashing_protection: 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] fn the_default_graffiti_is_empty() { let bytes = options("").graffiti().expect("valid"); diff --git a/crates/net/rpc/src/beacon/proposal.rs b/crates/net/rpc/src/beacon/proposal.rs index df520af4..49715970 100644 --- a/crates/net/rpc/src/beacon/proposal.rs +++ b/crates/net/rpc/src/beacon/proposal.rs @@ -1254,6 +1254,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}; @@ -1280,28 +1281,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) @@ -1697,116 +1676,14 @@ mod tests { assert_eq!(network.blocks.lock().unwrap().len(), 2); } - 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())) } @@ -2624,3 +2501,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 56ffc9b0..88c8a795 100644 --- a/crates/net/rpc/src/beacon/validator_client_tests.rs +++ b/crates/net/rpc/src/beacon/validator_client_tests.rs @@ -129,7 +129,7 @@ async fn serve_at( // keep the state under that root too, as a real anchor would be. let parent_root = real_head_root(&state); 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) } @@ -148,16 +148,21 @@ fn real_head_root(state: &BeaconState) -> ethlambda_types::primitives::H256 { /// 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( store: Store, engine: Option, + builder: Option>, ) -> ( HttpBeaconNode, Arc, SharedPayloadAttestationPool, std::net::SocketAddr, ) { - spawn_server_sharing(store, engine, SharedSyncCommitteePool::default()).await + let sync_pool = SharedSyncCommitteePool::default(); + spawn_server_sharing(store, engine, sync_pool, builder).await } /// [`spawn_server`] pooling sync committee messages in `sync_pool`, so a second @@ -168,6 +173,7 @@ async fn spawn_server_sharing( mut store: Store, engine: Option, sync_pool: SharedSyncCommitteePool, + builder: Option>, ) -> ( HttpBeaconNode, Arc, @@ -198,6 +204,10 @@ async fn spawn_server_sharing( .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(); @@ -420,6 +430,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:?}"); @@ -660,6 +671,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); @@ -694,6 +706,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"), } } @@ -924,7 +938,7 @@ async fn serve_fulu_pinned( head_root, slot + clock_ahead, ); - let (client, network, _, _) = spawn_server_sharing(store, engine, sync_pool).await; + let (client, network, _, _) = spawn_server_sharing(store, engine, sync_pool, None).await; let genesis = client.genesis().await.expect("genesis"); let spec = client .spec() @@ -1119,6 +1133,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)); @@ -1240,7 +1255,7 @@ mod gloas { let store = beacon_store_with_head_block(state.clone(), config(), block, head_root, clock_slot); let (client, network, payload_pool, _address) = - spawn_server_sharing(store.clone(), engine, sync_pool).await; + spawn_server_sharing(store.clone(), engine, sync_pool, None).await; let genesis = client.genesis().await.expect("genesis"); let spec = client .spec() @@ -1570,6 +1585,7 @@ mod gloas { ), ), ethlambda_validator::slashing_protection::SlashingProtection::in_memory(), + None, ); let duty = ProposerDutyDto { pubkey: encode_hex(&pubkey.0), @@ -1699,6 +1715,7 @@ mod gloas { ), ), ethlambda_validator::slashing_protection::SlashingProtection::in_memory(), + None, ); let duty = ProposerDutyDto { pubkey: encode_hex(&pubkey.0), @@ -1718,3 +1735,310 @@ 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 (anchor_root, 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(); + // The publish endpoints refuse a parent at the finalized slot other + // than the finalized block, so finalize the root the block names, as a + // real anchor's checkpoint does. + crate::test_utils::finalize_at(&mut store, anchor_root, parent_root, head_state.slot()); + + 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) + } + + /// Registered, a bid taken at the maximum factor, and the blinded + /// block signed: its root and the body to publish. + async fn signed_blinded(&self) -> (ethlambda_types::primitives::H256, Vec) { + self.register().await; + self.world.offer(1, |_| {}); + let produced = self.produce(Some(u64::MAX)).await; + assert!(produced.is_blinded()); + let block_root = produced.block_root(); + (block_root, self.sign(produced).await) + } + + async fn publish_blinded( + &self, + block_root: ethlambda_types::primitives::H256, + body: &[u8], + ) -> ethlambda_validator::Result { + self.client + .publish_blinded_block(self.world.slot, block_root, ForkName::Fulu, body) + .await + } + } + + /// A relay refusing the block is a definite refusal at the client: the + /// node passes the builder's 400 through. + #[tokio::test] + async fn a_relay_refusing_the_blinded_block_is_a_definite_refusal() { + let world = builder_world(false, SyncStatus::Synced).await; + world + .relay + .answer_submissions_with(axum::http::StatusCode::BAD_REQUEST); + let engine = fake_execution_client_paying("0x2a", false).await; + let h = harness(world, engine).await; + + let (block_root, body) = h.signed_blinded().await; + let err = h + .publish_blinded(block_root, &body) + .await + .expect_err("the relay refused it"); + assert!( + matches!(err, Error::BeaconNodeStatus { status: 400, .. }), + "got {err:?}" + ); + } + + /// A relay failing without saying whether it published is unconfirmed at + /// the client, not a missed proposal: the relay may still publish. + #[tokio::test] + async fn a_relay_failing_on_the_blinded_block_leaves_it_unconfirmed() { + let world = builder_world(false, SyncStatus::Synced).await; + world + .relay + .answer_submissions_with(axum::http::StatusCode::INTERNAL_SERVER_ERROR); + let engine = fake_execution_client_paying("0x2a", false).await; + let h = harness(world, engine).await; + + let (block_root, body) = h.signed_blinded().await; + let err = h + .publish_blinded(block_root, &body) + .await + .expect_err("the relay did not confirm it"); + assert!( + matches!( + err, + Error::BlindedUnconfirmed { + status: Some(500), + .. + } + ), + "got {err:?}" + ); + } + + /// Once the node handed a blinded block to its builder, it refuses a + /// different block for the slot from the same client, as a second + /// signature for it would be an equivocation. + #[tokio::test] + async fn the_node_refuses_a_second_block_after_a_blinded_one() { + let world = builder_world(false, SyncStatus::Synced).await; + let engine = fake_execution_client_paying("0x2a", false).await; + let h = harness(world, engine).await; + + let (block_root, body) = h.signed_blinded().await; + let published = h.publish_blinded(block_root, &body).await.expect("handed"); + assert_eq!(published, Published::HandedToBuilder); + + let full = h.produce(None).await; + assert!(!full.is_blinded()); + let full = h.sign(full).await; + let err = h + .client + .publish_block(ForkName::Fulu, &full) + .await + .expect_err("the slot is claimed by the blinded block"); + assert!( + matches!(&err, Error::BeaconNodeStatus { status: 400, body } + if body.contains("already published a different block")), + "got {err:?}" + ); + } + + #[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 block_root = produced.block_root(); + let body = h.sign(produced).await; + let published = h + .client + .publish_blinded_block(h.world.slot, block_root, 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/Cargo.toml b/crates/validator/Cargo.toml index b34e7248..56b24478 100644 --- a/crates/validator/Cargo.toml +++ b/crates/validator/Cargo.toml @@ -42,6 +42,8 @@ blst = "0.3.16" # the `impl Future + Send` bounds at every method is noise this trait does not # need. async-trait = "0.1" +# `join_all`, so the calls every beacon node must hear reach them all at once. +futures-util = "0.3" # EIP-2335 keystore decryption. scrypt = { version = "0.11", default-features = false } diff --git a/crates/validator/src/beacon_node/block_contents.rs b/crates/validator/src/beacon_node/block_contents.rs index c4d2806c..c46341a2 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 slashing protection 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 f05107ab..7bebcc6d 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,29 @@ type BoxFuture<'a, T> = Pin + Send + 'a>>; /// first one that succeeds. pub struct FallbackBeaconNode { nodes: Vec, + /// The latest blinded block produced, and the node that produced it. 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>, +} + +/// Which node produced which blinded block. +#[derive(Debug, Clone, Copy)] +struct BlindedProducer { + slot: Slot, + /// Checked at publish, so the record answers for the block being + /// published rather than for whatever was last produced at its slot. + block_root: Root, + position: usize, } 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 +86,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,17 +140,34 @@ 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<()> + /// + /// The nodes are called at once rather than in turn, as Lighthouse + /// broadcasts, so the call takes as long as its slowest node rather than + /// the sum of every node's timeout: a hung node no longer holds back the + /// healthy ones behind it in the list. + /// + /// `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<()>>, { + let answers = futures_util::future::join_all(self.nodes.iter().map(&call)).await; let mut accepted = 0; let mut last = None; - for (position, node) in self.nodes.iter().enumerate() { - match call(node).await { + for (position, answer) in answers.into_iter().enumerate() { + match answer { 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 +178,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 +277,27 @@ impl BeaconNodeApi for FallbackBeaconNode { .await } + /// Remembers which node answered when the block is blinded, and forgets + /// it otherwise, a production that failed on every node included: the + /// record is for the latest block only, so nothing 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 produced = self + .try_each_at("produce_block", |node| node.produce_block(request)) + .await; + let record = match &produced { + Ok((position, block)) if block.is_blinded() => Some(BlindedProducer { + slot: block.slot(), + block_root: block.block_root(), + position: *position, + }), + _ => None, + }; + *self + .blinded_producer + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) = record; + produced.map(|(_, block)| block) } /// First node that accepts it wins, rather than a broadcast to every node. @@ -253,6 +318,56 @@ 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. Slashing + /// protection has already recorded the slot, so nothing retries it. + async fn publish_blinded_block( + &self, + slot: Slot, + block_root: Root, + fork: ForkName, + body: &[u8], + ) -> Result { + let record = *self + .blinded_producer + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + match record { + Some(producer) if producer.slot == slot && producer.block_root == block_root => { + self.nodes[producer.position] + .publish_blinded_block(slot, block_root, fork, body) + .await + } + _ => Err(Error::InconsistentResponse(format!( + "no beacon node produced blinded block {} for slot {slot}", + crate::beacon_node::dto::encode_hex(&block_root.0) + ))), + } + } + + /// 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 +464,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 +478,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 +544,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 +725,7 @@ mod tests { proposer_index, randao_reveal: BlsSignature([0; 96]), graffiti: Bytes32::default(), + builder_boost_factor: None, } } @@ -850,4 +975,207 @@ 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, produced.block_root(), 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]); + + let produced = fallback + .produce_block(&blinded_request(5, 7)) + .await + .expect("node 1 produces"); + let err = fallback + .publish_blinded_block(5, produced.block_root(), 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" + ); + } + + /// The record answers for one block: another slot, or another root at the + /// same slot, is refused without reaching any node. + #[tokio::test] + async fn a_blinded_publish_for_another_slot_or_block_is_refused() { + let fallback = FallbackBeaconNode::new(vec![blinded_node()]); + let produced = fallback + .produce_block(&blinded_request(5, 7)) + .await + .expect("produces"); + let root = produced.block_root(); + + for (slot, block_root, what) in [ + (6, root, "nothing was produced for slot 6"), + (5, Root::repeat_byte(0xee), "that block was not produced"), + ] { + let err = fallback + .publish_blinded_block(slot, block_root, ForkName::Electra, &[1]) + .await + .expect_err(what); + assert!(matches!(err, Error::InconsistentResponse(_)), "got {err:?}"); + } + assert!( + fallback.nodes[0] + .published_blinded + .lock() + .expect("lock") + .is_empty() + ); + } + + /// A production that fails on every node clears the record too, so the + /// earlier block's claim does not outlive the attempt that replaced it. + #[tokio::test] + async fn a_failed_production_clears_the_blinded_record() { + let fallback = FallbackBeaconNode::new(vec![blinded_node()]); + let produced = fallback + .produce_block(&blinded_request(5, 7)) + .await + .expect("blinded"); + fallback + .produce_block(&blinded_request(6, 7)) + .await + .expect_err("the node holds no block for slot 6"); + + fallback + .publish_blinded_block(5, produced.block_root(), ForkName::Electra, &[1]) + .await + .expect_err("the record is gone"); + } + + /// 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()]); + let blinded = 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, blinded.block_root(), 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() + ); + } + + /// Every node is called at once: two slow nodes cost one delay, not the + /// sum, so a hung node does not hold back the ones behind it. + #[tokio::test] + async fn every_node_is_called_at_once() { + let delay = std::time::Duration::from_millis(300); + let slow = || MockBeaconNode::new().with_registration_delay(delay); + let fallback = FallbackBeaconNode::new(vec![slow(), slow()]); + + let started = std::time::Instant::now(); + fallback + .register_validators(&[]) + .await + .expect("both accept"); + let elapsed = started.elapsed(); + + assert!(elapsed < delay * 2, "took {elapsed:?}"); + for node in &fallback.nodes { + assert_eq!(*node.registration_batches.lock().expect("lock"), vec![0]); + } + } } diff --git a/crates/validator/src/beacon_node/http.rs b/crates/validator/src/beacon_node/http.rs index 013964c7..4c8abe53 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,64 @@ 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); + +/// How much of a failed answer's body is read. Enough for a pool submission's +/// 400, which lists each refused entry and is parsed for them. +const ERROR_BODY_READ_LIMIT: usize = 1024 * 1024; + +/// How much of a failed answer's body an error carries, and so a log line. +const ERROR_TEXT_LIMIT: usize = 1024; + +/// A failed answer's body, read up to [`ERROR_BODY_READ_LIMIT`] bytes, so a +/// node answering with an endless body cannot hold the client on it. +async fn read_error_body(mut response: reqwest::Response) -> String { + let mut bytes = Vec::new(); + while bytes.len() < ERROR_BODY_READ_LIMIT { + match response.chunk().await { + Ok(Some(chunk)) => bytes.extend_from_slice(&chunk), + _ => break, + } + } + bytes.truncate(ERROR_BODY_READ_LIMIT); + String::from_utf8_lossy(&bytes).into_owned() +} + +/// A body as an error shows it: control characters written out (`\n`, `\r`, +/// `\u{1b}`, ...), so the log line it lands in stays one line and carries no +/// terminal escapes, and cut to [`ERROR_TEXT_LIMIT`] bytes. +fn error_text(body: String) -> String { + let mut text = String::with_capacity(body.len()); + for c in body.chars() { + // Every control character, and the two Unicode line separators, is + // written out, so neither a line break nor a terminal escape sequence + // reaches the log as itself. + if c.is_control() || matches!(c, '\u{2028}' | '\u{2029}') { + text.extend(c.escape_default()); + } else { + text.push(c); + } + } + if text.len() > ERROR_TEXT_LIMIT { + let mut end = ERROR_TEXT_LIMIT; + while !text.is_char_boundary(end) { + end -= 1; + } + text.truncate(end); + text.push_str("..."); + } + text +} + +/// [`read_error_body`] as [`error_text`] shows it. +async fn error_body(response: reqwest::Response) -> String { + error_text(read_error_body(response).await) +} + /// 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 +205,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), @@ -161,13 +236,37 @@ impl HttpBeaconNode { if status.is_success() { return Ok(()); } - let body = response.text().await.unwrap_or_default(); + let body = error_body(response).await; Err(Error::BeaconNodeStatus { status: status.as_u16(), body, }) } + /// 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 @@ -236,7 +335,7 @@ impl HttpBeaconNode { return Err(Error::BeaconNodeSyncing); } if !status.is_success() { - let body = response.text().await.unwrap_or_default(); + let body = error_body(response).await; return Err(Error::BeaconNodeStatus { status: status.as_u16(), body, @@ -446,7 +545,7 @@ impl HttpBeaconNode { Err(Error::BeaconNodeStatus { status: status.as_u16(), - body, + body: error_text(body), }) } @@ -456,7 +555,7 @@ impl HttpBeaconNode { return Err(Error::BeaconNodeSyncing); } if !status.is_success() { - let body = response.text().await.unwrap_or_default(); + let body = error_body(response).await; return Err(Error::BeaconNodeStatus { status: status.as_u16(), body, @@ -593,8 +692,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 +707,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 slashing-protection + /// record 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 +741,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 +763,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 { @@ -680,13 +779,97 @@ impl BeaconNodeApi for HttpBeaconNode { if status.is_success() { return Ok(Published::Imported); } - let body = response.text().await.unwrap_or_default(); + let body = error_body(response).await; + Err(Error::BeaconNodeStatus { + status: status.as_u16(), + body, + }) + } + + /// 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. + /// + /// The status alone cannot always tell, and two edges are knowingly read + /// the wrong way. Some of ethlambda's own 500s are definite (no builder + /// configured, the blinded block failing the node's checks) and are read as + /// unconfirmed. A 503 from a proxy that had already forwarded the request + /// is read as definite. Neither changes what happens next: nothing is + /// retried and nothing is signed again either way. Only the log line and + /// the counter it lands on are wrong. + async fn publish_blinded_block( + &self, + // For the fallback, which picks the node that produced this block; + // one node has nothing to choose. + _slot: Slot, + _block_root: Root, + 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 = error_body(response).await; + 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, @@ -744,7 +927,7 @@ impl BeaconNodeApi for HttpBeaconNode { if status.is_success() { return Ok(()); } - let body = response.text().await.unwrap_or_default(); + let body = error_body(response).await; Err(Error::BeaconNodeStatus { status: status.as_u16(), body, @@ -813,7 +996,7 @@ impl BeaconNodeApi for HttpBeaconNode { if status.is_success() { return Ok(messages.len()); } - let body = response.text().await.unwrap_or_default(); + let body = read_error_body(response).await; Self::handle_pool_submission(status, body, messages.len()) } @@ -843,7 +1026,7 @@ impl BeaconNodeApi for HttpBeaconNode { if status.is_success() { return Ok(attestations.len()); } - let body = response.text().await.unwrap_or_default(); + let body = read_error_body(response).await; Self::handle_pool_submission(status, body, attestations.len()) } @@ -1008,7 +1191,7 @@ impl BeaconNodeApi for HttpBeaconNode { if status.is_success() { return Ok(messages.len()); } - let body = response.text().await.unwrap_or_default(); + let body = read_error_body(response).await; Self::handle_pool_submission(status, body, messages.len()) } @@ -1189,6 +1372,274 @@ 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()); + } + + /// A node's error body reaches the logs on one line and cut short: a body + /// with line breaks cannot forge log lines, and a long one cannot flood + /// them. + #[tokio::test] + async fn an_error_body_is_one_line_and_capped() { + let reply = "first line\nsecond line\r\n".repeat(200).into_bytes(); + let served = serve(400, vec![], reply).await; + let err = served.node.genesis().await.expect_err("a 400"); + let Error::BeaconNodeStatus { status, body } = err else { + panic!("got {err:?}"); + }; + assert_eq!(status, 400); + assert!(body.starts_with("first line\\nsecond line\\r\\nfirst line")); + assert!(!body.contains('\n') && !body.contains('\r')); + assert_eq!(body.len(), ERROR_TEXT_LIMIT + "...".len()); + } + + #[test] + fn error_text_cuts_on_a_character_boundary() { + let text = error_text("é".repeat(ERROR_TEXT_LIMIT)); + assert!(text.ends_with("...")); + assert!(text.len() <= ERROR_TEXT_LIMIT + "...".len()); + assert_eq!(error_text("short".to_string()), "short"); + } + + #[test] + fn error_text_writes_out_every_control_character() { + let text = error_text("a\u{1b}[31mb\tc\u{2028}d".to_string()); + assert_eq!(text, "a\\u{1b}[31mb\\tc\\u{2028}d"); + } + + #[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, Root::ZERO, 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, Root::ZERO, 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, Root::ZERO, 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, Root::ZERO, 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, Root::ZERO, 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 fe4a952b..f91284a8 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,35 @@ 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, + /// With [`Self::produces_blinded`], answer with the blinded block even + /// when the request carries no factor: a node breaking the contract, so a + /// test can see what the caller does with one. + pub blinded_regardless: 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>, + /// How long `register_validators` takes before it records the batch, as + /// a slow node would. + pub registration_delay: Option, + /// 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 +288,47 @@ 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 + } + + /// Answer every pre-gloas `produce_block` with a blinded block, factor or + /// not, as a node breaking the contract on `BeaconNodeApi::produce_block`. + pub fn with_blinded_block_regardless(mut self) -> Self { + self.produces_blinded = true; + self.blinded_regardless = 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 `register_validators` take `delay` before it records the batch. + pub fn with_registration_delay(mut self, delay: std::time::Duration) -> Self { + self.registration_delay = Some(delay); + 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 +711,18 @@ impl BeaconNodeApi for MockBeaconNode { failure: BeaconNodeFailure::Request, detail: "no block set".into(), })?; + if self.produces_blinded + && (request.builder_boost_factor.is_some() || self.blinded_regardless) + { + 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 +747,63 @@ impl BeaconNodeApi for MockBeaconNode { Ok(self.publish_outcome.unwrap_or(Published::Imported)) } + async fn publish_blinded_block( + &self, + slot: Slot, + _block_root: Root, + 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")?; + if let Some(delay) = self.registration_delay { + tokio::time::sleep(delay).await; + } + 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 49c1c54d..860f033c 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; @@ -142,6 +143,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. @@ -157,6 +163,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. @@ -364,12 +378,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 @@ -385,6 +400,37 @@ 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, the one with root `block_root` 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, + block_root: Root, + 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 ae65aee9..05065123 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. /// @@ -161,6 +178,7 @@ impl Error { | Self::AllBeaconNodesFailed(_) | Self::BeaconNodeStatus { .. } | Self::BeaconNodeSyncing + | Self::BlindedUnconfirmed { .. } | Self::InconsistentResponse(_) | Self::AttestationDeadline { .. } | Self::Decode(_) => true, diff --git a/crates/validator/src/in_flight.rs b/crates/validator/src/in_flight.rs new file mode 100644 index 00000000..ffbac8b2 --- /dev/null +++ b/crates/validator/src/in_flight.rs @@ -0,0 +1,68 @@ +//! One run at a time for work spawned off the slot loop. +//! +//! The slot loop spawns its sends to beacon nodes rather than awaiting them, so +//! a slow node cannot delay the next slot's duties. The cost is that nothing +//! stops a new run starting while the last one still waits on that node. Each +//! run holds its own copy of the work and sends it again, so the runs pile up +//! for as long as the node stays slow. A run that finds the previous one still +//! going skips instead, as Teku's registration round does. + +use std::sync::atomic::{AtomicBool, Ordering}; + +/// Whether a run is going. +#[derive(Debug, Default)] +pub(crate) struct InFlight(AtomicBool); + +impl InFlight { + /// The guard for a new run, or `None` when one is already going. + pub(crate) fn enter(&self) -> Option> { + let running = self.0.swap(true, Ordering::AcqRel); + // The guard is built only for a run that starts: `then_some` would + // build it either way and drop it on the refused path, and dropping + // it clears the flag of the run that is still going. + if running { + None + } else { + Some(InFlightGuard(&self.0)) + } + } +} + +/// Ends the run on every exit from it, a panic included, so one failed run +/// cannot stop every later one. +pub(crate) struct InFlightGuard<'a>(&'a AtomicBool); + +impl Drop for InFlightGuard<'_> { + fn drop(&mut self) { + self.0.store(false, Ordering::Release); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_second_run_is_refused_until_the_first_ends() { + let in_flight = InFlight::default(); + let first = in_flight.enter().expect("nothing is running"); + assert!(in_flight.enter().is_none()); + assert!( + in_flight.enter().is_none(), + "a refused run leaves the running one's flag set" + ); + drop(first); + assert!(in_flight.enter().is_some()); + } + + #[test] + fn a_run_that_panics_still_ends() { + let in_flight = InFlight::default(); + let unwound = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + let _run = in_flight.enter().expect("nothing is running"); + panic!("the run fails"); + })); + assert!(unwound.is_err()); + assert!(in_flight.enter().is_some()); + } +} diff --git a/crates/validator/src/lib.rs b/crates/validator/src/lib.rs index 24fc99df..4017122e 100644 --- a/crates/validator/src/lib.rs +++ b/crates/validator/src/lib.rs @@ -31,11 +31,13 @@ pub mod beacon_node; pub mod duties; pub mod error; pub mod http_api; +pub(crate) mod in_flight; pub mod keys; pub mod metrics; pub mod payload_attestation; pub mod proposal; pub mod proposer_settings; +pub mod registration; pub(crate) mod secure_fs; pub mod signing; pub mod slashing_protection; @@ -55,15 +57,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; @@ -89,12 +92,24 @@ 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, /// Register keys that have no slashing-protection record as never having /// signed, instead of refusing to start. See /// [`slashing_protection::SlashingProtection::open`]. pub init_slashing_protection: bool, } +/// 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. pub async fn run(config: ValidatorConfig) -> Result<()> { metrics::init(); @@ -112,6 +127,18 @@ pub async fn run(config: ValidatorConfig) -> Result<()> { keymanager API." ); } + // Accepted, as every other client's flag accepts it, but said loudly: + // nobody holds the zero address's key, so what it is paid is burned, and + // a builder registration or proposer preference signs that into a + // commitment the builder honours. Only the default can be zero, since the + // keymanager API refuses it for a single validator. + if config.suggested_fee_recipient == Some(ExecutionAddress::ZERO) { + error!( + "--suggested-fee-recipient is the zero address. Every validator without its own \ + address burns its execution-layer rewards, and builder registrations and proposer \ + preferences sign that address for the builder to pay." + ); + } let settings = Arc::new(ProposerSettings::new( config.graffiti, config.suggested_fee_recipient, @@ -214,7 +241,20 @@ pub async fn run(config: ValidatorConfig) -> Result<()> { context.clone(), settings.clone(), protection.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)?; @@ -290,7 +330,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 @@ -345,6 +397,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())); + } } } @@ -640,7 +699,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, @@ -650,16 +709,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, @@ -674,6 +754,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, @@ -682,12 +765,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? { @@ -758,7 +841,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 ad52aa95..92e1466b 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 slashing protection judged /// them unsafe, or could not record them. /// @@ -341,6 +381,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); @@ -407,6 +451,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 c16319f0..3a7f0931 100644 --- a/crates/validator/src/proposal.rs +++ b/crates/validator/src/proposal.rs @@ -42,6 +42,28 @@ //! //! The envelope is not slashable, so slashing protection 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 +//! slashing protection, 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 @@ -52,6 +74,7 @@ //! than dangerous, but the caller should not be relying on that. use std::sync::Arc; +use std::time::SystemTime; use ethlambda_types::beacon::constants::BUILDER_INDEX_SELF_BUILD; use ethlambda_types::beacon::fork::ForkName; @@ -68,6 +91,7 @@ use crate::keys::ValidatorStore; use crate::proposer_settings::ProposerSettings; use crate::signing::SigningContext; use crate::slashing_protection::{Refusal, SharedProtection}; +use crate::slot_clock::SlotClock; pub struct ProposalService { beacon_node: Arc, @@ -82,23 +106,66 @@ pub struct ProposalService { /// What every validator has signed, shared with the attestation service /// and the keymanager. See [`crate::slashing_protection`]. protection: SharedProtection, + /// 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` because the critical section + /// is a single store with no await inside it. + 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, protection: SharedProtection, + builder_boost_factor: Option, ) -> Self { Self { beacon_node, context, settings, protection, + builder_boost_factor, + last_blinded_slot: std::sync::Mutex::new(None), + clock: None, } } + /// 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. /// @@ -192,6 +259,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?; @@ -209,6 +277,17 @@ impl ProposalService { // second block for a slot already proposed. validate_produced_block(&request, &produced)?; + // Also the implementation's contract, and checked again for the same + // reason. With builder proposals off this client never publishes a + // blinded block, so signing one would record the slot in slashing + // protection for a block that can never be sent. + if produced.is_blinded() && self.builder_boost_factor.is_none() { + return Err(Error::InconsistentResponse(format!( + "node produced a blinded block for slot {slot}; builder proposals are off, so \ + this client cannot publish one" + ))); + } + // The node's fork and this client's must agree, or the signature is // computed under a fork version the network does not accept. // @@ -229,13 +308,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" ); @@ -258,7 +347,16 @@ impl ProposalService { let signature = { let store = store.read().await; - 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 @@ -268,10 +366,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, block_root, 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) { @@ -366,6 +472,109 @@ 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, + block_root: Root, + 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, block_root, 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 @@ -397,6 +606,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) } @@ -483,6 +696,7 @@ mod tests { context(), Arc::new(settings), SlashingProtection::in_memory(), + None, ) } @@ -496,6 +710,7 @@ mod tests { context(), Arc::new(settings), SlashingProtection::in_memory(), + None, ) } @@ -598,6 +813,7 @@ mod tests { context(), settings.clone(), SlashingProtection::in_memory(), + None, ); settings.set_graffiti(&pubkey, Bytes32::repeat_byte(0xcd)); @@ -684,14 +900,20 @@ mod tests { }; let node = Arc::new(MockBeaconNode::new().with_block(slot(), 7)); - ProposalService::new(node.clone(), context(), settings.clone(), protection()) - .propose(slot(), &duty(&pubkey, 7, slot()), &store) - .await - .expect("first"); + ProposalService::new( + node.clone(), + context(), + settings.clone(), + protection(), + None, + ) + .propose(slot(), &duty(&pubkey, 7, slot()), &store) + .await + .expect("first"); assert_eq!(node.published_blocks().len(), 1); let node_again = Arc::new(MockBeaconNode::new().with_block(slot(), 7)); - let err = ProposalService::new(node_again.clone(), context(), settings, protection()) + let err = ProposalService::new(node_again.clone(), context(), settings, protection(), None) .propose(slot(), &duty(&pubkey, 7, slot()), &store) .await .expect_err("the slot was proposed before the restart"); @@ -717,6 +939,7 @@ mod tests { context(), settings, Arc::new(std::sync::Mutex::new(opened)), + None, ) .propose(slot(), &duty(&pubkey, 7, slot()), &store) .await @@ -745,6 +968,300 @@ 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), + SlashingProtection::in_memory(), + 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("slashing protection 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" + ); + + // Slashing protection recorded the slot before the publish started, so a retry + // after the dropped future is refused without asking for a block. + let err = service + .propose(slot(), &duty(&pubkey, 7, slot()), &store) + .await + .expect_err("the slot is signed"); + assert!(matches!(err, Error::ProposalRefused { .. }), "got {err:?}"); + assert_eq!(node.block_requests().len(), 1); + } + + /// 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 node breaking its contract with a blinded answer to a request with no + /// factor is refused before signing. With builder proposals off this client + /// never publishes a blinded block, so signing would burn the slot for + /// nothing. The second attempt is refused for the same reason rather than + /// by slashing protection, which shows nothing was recorded. + #[tokio::test] + async fn a_blinded_block_is_refused_before_signing_with_builder_proposals_off() { + let (store, pubkey) = store(); + let node = Arc::new( + MockBeaconNode::new() + .with_block(slot(), 7) + .with_blinded_block_regardless(), + ); + let service = service(node.clone()); + + for attempt in 0..2 { + let err = service + .propose(slot(), &duty(&pubkey, 7, slot()), &store) + .await + .expect_err("a blinded block must not be signed"); + assert!( + matches!(err, Error::InconsistentResponse(_)), + "attempt {attempt}: got {err:?}" + ); + } + assert!(!service.signed_blinded_at(slot())); + assert!(node.published_blinded.lock().expect("lock").is_empty()); + assert!(node.published_blocks().is_empty()); + } + + /// The signature in the published blinded block verifies over the full + /// block's root. The blinded container's root equals it, which is what lets + /// the relay put the payload back under a signature over a block the + /// validator never saw whole. + #[tokio::test] + async fn a_blinded_block_is_signed_over_the_full_blocks_root() { + use crate::beacon_node::block_contents::empty_block_for; + use blst::min_pk::{PublicKey, Signature}; + use ethlambda_types::beacon::builder::SignedBlindedBeaconBlock; + + let (store, pubkey) = store(); + let node = Arc::new( + MockBeaconNode::new() + .with_block(slot(), 7) + .with_blinded_block(), + ); + builder_service(node.clone(), None) + .propose(slot(), &duty(&pubkey, 7, slot()), &store) + .await + .expect("proposes"); + + let (_, _, body) = node.published_blinded.lock().expect("lock").remove(0); + let published = SignedBlindedBeaconBlock::from_ssz_bytes(&body).expect("decodes"); + let full_root = empty_block_for(slot(), 7).hash_tree_root(); + assert_eq!(published.message.hash_tree_root(), full_root); + + let root = context().block_signing_root(full_root, slot()); + let pk = PublicKey::from_bytes(&pubkey.0).expect("valid pubkey"); + let sig = Signature::from_bytes(&published.signature.0).expect("valid signature"); + assert_eq!( + sig.verify( + true, + root.as_slice(), + b"BLS_SIG_BLS12381G2_XMD:SHA-256_SSWU_RO_POP_", + &[], + &pk, + true + ), + blst::BLST_ERROR::BLST_SUCCESS + ); + } + + /// A blinded answer is held to what a full one is: the slot and proposer + /// asked for, and the fork this client's schedule puts the slot in. Each + /// is refused before signing, so none spends the slot. + #[tokio::test] + async fn a_blinded_block_for_another_slot_proposer_or_fork_is_refused() { + let (store, pubkey) = store(); + // The mock always names electra, and slot 96 is phase0 on mainnet. + let cases = [ + ("slot", (slot() + 1, 7), slot()), + ("proposer", (slot(), 8), slot()), + ("fork", (96, 7), 96), + ]; + for (what, (block_slot, proposer), requested) in cases { + let node = Arc::new( + MockBeaconNode::new() + .with_block(block_slot, proposer) + .with_blinded_block(), + ); + let service = builder_service(node.clone(), None); + let err = service + .propose(requested, &duty(&pubkey, 7, requested), &store) + .await + .expect_err(what); + assert!( + matches!(err, Error::InconsistentResponse(_)), + "{what}: got {err:?}" + ); + assert!(!service.signed_blinded_at(requested), "{what}"); + assert!( + node.published_blinded.lock().expect("lock").is_empty(), + "{what}" + ); + } + } + /// A duty naming a key this client does not hold must fail before anything /// is asked of the beacon node. #[tokio::test] @@ -908,6 +1425,7 @@ mod gloas_tests { context(), Arc::new(settings), SlashingProtection::in_memory(), + None, ) } @@ -990,6 +1508,7 @@ mod gloas_tests { context(), settings, SlashingProtection::in_memory(), + None, ) .propose(slot(), &duty(&pubkey), &store) .await diff --git a/crates/validator/src/proposer_settings.rs b/crates/validator/src/proposer_settings.rs index 01daf3f4..2d73f439 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 @@ -138,6 +153,29 @@ impl ProposerSettings { }) } + /// `pubkey`'s fee recipient and gas limit read together, under one lock, or + /// `None` when it has no fee recipient. + /// + /// For a signed message that carries both (a builder registration, proposer + /// preferences): two separate reads could straddle a keymanager write and + /// pair an old address with a new gas limit, a combination the operator + /// never configured, which the builder then keeps. + pub fn fee_recipient_and_gas_limit( + &self, + pubkey: &BlsPubkey, + ) -> Option<(ExecutionAddress, u64)> { + self.read(|inner| { + let overrides = inner.overrides.get(pubkey); + let fee_recipient = overrides + .and_then(|o| o.fee_recipient) + .or(inner.default_fee_recipient)?; + let gas_limit = overrides + .and_then(|o| o.gas_limit) + .unwrap_or(inner.default_gas_limit); + Some((fee_recipient, gas_limit)) + }) + } + /// The graffiti `pubkey` proposes with. pub fn graffiti(&self, pubkey: &BlsPubkey) -> Bytes32 { self.read(|inner| { @@ -183,6 +221,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); } } @@ -223,6 +262,28 @@ mod tests { assert_eq!(settings.gas_limit(&key(2)), DEFAULT_GAS_LIMIT); } + /// The pair a signed registration or preference carries agrees with the + /// two separate reads, and is `None` exactly when there is no address. + #[test] + fn the_fee_recipient_and_gas_limit_read_together_match_the_single_reads() { + let settings = ProposerSettings::new(Bytes32::ZERO, None); + assert_eq!(settings.fee_recipient_and_gas_limit(&key(1)), None); + + settings.set_gas_limit(&key(1), 30_000_000); + assert_eq!(settings.fee_recipient_and_gas_limit(&key(1)), None); + settings.set_fee_recipient(&key(1), H160([0xbb; 20])); + assert_eq!( + settings.fee_recipient_and_gas_limit(&key(1)), + Some((H160([0xbb; 20]), 30_000_000)) + ); + + let defaults = self::settings(); + assert_eq!( + defaults.fee_recipient_and_gas_limit(&key(2)), + Some((H160([0xaa; 20]), DEFAULT_GAS_LIMIT)) + ); + } + /// Clearing one field leaves the others overridden. #[test] fn clearing_an_override_restores_that_default_only() { diff --git a/crates/validator/src/registration.rs b/crates/validator/src/registration.rs new file mode 100644 index 00000000..a33cf529 --- /dev/null +++ b/crates/validator/src/registration.rs @@ -0,0 +1,693 @@ +//! 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. +//! +//! A batch a node refuses with a 400 is not split to find the entry it +//! objects to: every entry is signed here by keys this client holds, so a +//! refusal says more about the node or the relay than about one entry, and no +//! other client splits either. +//! +//! # One send of each kind at a time +//! +//! Both sends are spawned off the slot loop, so nothing else would stop a new +//! one starting while the last still waits on a slow node: a full send every +//! epoch, each holding the whole registration set and sending it again to the +//! healthy nodes. A send that finds the previous one of its kind still going +//! is skipped (see [`crate::in_flight`]). A skipped full send waits for the +//! next epoch, and a skipped per-slot one for the next slot, since it has not +//! marked its change as sent. +//! +//! # 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::in_flight::InFlight; +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; + } +} + +/// The timestamp for a registration replacing `held`: `now_secs`, or one +/// second past `held`'s when the clock does not say later. +/// +/// A relay keeps a validator's registration with the latest timestamp and +/// drops one that is not newer. Two settings changes within one second, or a +/// wall clock stepped back between them, would otherwise sign the new settings +/// under a timestamp the relay ignores. The cached registration is then re-sent +/// every epoch while the builder keeps the old fee recipient or gas limit. +/// +/// Two changes within a second put it one second ahead of the clock, which no +/// relay refuses as being in the future. After a larger backward step it is +/// further ahead: a relay may refuse it until its own clock catches up, and the +/// epoch's full re-send then delivers it. With the clock's own reading +/// instead, the relay would drop it as older on every re-send, since the +/// cached signature keeps that timestamp until the settings change again. +fn newer_than(held: Option<&SignedValidatorRegistrationV1>, now_secs: u64) -> u64 { + held.map_or(now_secs, |held| { + now_secs.max(held.message.timestamp.saturating_add(1)) + }) +} + +/// 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, + /// The epoch's full send, while it runs. + full_send: InFlight, + /// The per-slot send of changed keys, while it runs. + changed_send: InFlight, +} + +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), + full_send: InFlight::default(), + changed_send: InFlight::default(), + } + } + + /// 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, gas_limit)) = + self.settings.fee_recipient_and_gas_limit(pubkey) + else { + current.no_fee_recipient += 1; + continue; + }; + + let held = signed.get(pubkey); + if let Some(held) = held + && 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: newer_than(held, 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; + } + let Some(_running) = self.full_send.enter() else { + warn!( + epoch, + "The previous builder registration send is still running; skipping this epoch's" + ); + 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; + } + // Silently: the change stays unsent, so the next slot picks it up. + let Some(_running) = self.changed_send.enter() else { + 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"); + } + + /// A relay drops a registration that is not newer than the one it holds, + /// so a change signed within the same second, or after the clock stepped + /// back, still gets a later timestamp than the registration it replaces. + #[test] + fn a_re_signed_registration_is_always_newer_than_the_last() { + let (store, pubkeys) = store_with_keys(1); + let settings = settings(Some(H160([0xab; 20]))); + let (service, _) = service(Config::mainnet(), settings.clone()); + let first = service.current(&store, &pubkeys, 2_000); + assert_eq!(first.all[0].message.timestamp, 2_000); + + settings.set_gas_limit(&pubkeys[0], 30_000_000); + let same_second = service.current(&store, &pubkeys, 2_000); + assert_eq!(same_second.changed[0].message.timestamp, 2_001); + + settings.set_gas_limit(&pubkeys[0], 31_000_000); + let stepped_back = service.current(&store, &pubkeys, 1_000); + assert_eq!(stepped_back.changed[0].message.timestamp, 2_002); + + // Once the clock is ahead again, its reading is used. + settings.set_gas_limit(&pubkeys[0], 32_000_000); + let caught_up = service.current(&store, &pubkeys, 5_000); + assert_eq!(caught_up.changed[0].message.timestamp, 5_000); + } + + #[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] + ); + } + + /// A full send that is still waiting on a slow node makes the next one + /// skip, rather than both holding the set and sending it again; once it + /// ends, the next one runs. + #[tokio::test] + async fn an_epoch_send_still_running_makes_the_next_one_skip() { + let (store, pubkeys) = store_with_keys(2); + let node = Arc::new( + MockBeaconNode::new().with_registration_delay(std::time::Duration::from_millis(300)), + ); + let service = Arc::new(RegistrationService::new( + node.clone(), + context(Config::mainnet()), + settings(Some(H160([0xab; 20]))), + )); + let store = Arc::new(RwLock::new(store)); + + let slow = tokio::spawn( + service + .clone() + .register_all(0, pubkeys.clone(), store.clone()), + ); + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + tokio::time::timeout( + std::time::Duration::from_millis(100), + service + .clone() + .register_all(1, pubkeys.clone(), store.clone()), + ) + .await + .expect("skips at once rather than waiting on the node"); + slow.await.expect("the first send ends"); + assert_eq!( + *node.registration_batches.lock().expect("lock"), + vec![2], + "only the first send reached the node" + ); + + service.register_all(2, pubkeys, store).await; + assert_eq!(*node.registration_batches.lock().expect("lock"), vec![2, 2]); + } + + #[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 9317ab34..d1c9767c 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 @@ -507,6 +526,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 f230f745..9c7d9f2e 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -297,9 +297,12 @@ implementation's, so none of the [common flags](#common-flags) apply to it. | `--http-address` | `127.0.0.1` | Bind address for the metrics and keymanager servers | | `--metrics-port` | `5064` | Prometheus metrics port | | `--keymanager-port` | `5062` | Keymanager API port. Only bound with `--enable-keymanager` | -| `--suggested-fee-recipient` | none | Execution address to receive block rewards, `0x`-prefixed, for every validator the keymanager API gives no address of its own. Optional, and startup warns when it is absent: without it the beacon node picks an address, and it will not be yours | +| `--suggested-fee-recipient` | none | Execution address to receive block rewards, `0x`-prefixed, for every validator the keymanager API gives no address of its own. Optional, and startup warns when it is absent: without it the beacon node picks an address, and it will not be yours. The zero address is accepted, as every other client accepts it, but reported at error at startup: it burns the rewards, and builder registrations and proposer preferences sign it for the builder to pay | | `--graffiti` | empty | Text for the graffiti field of proposed blocks, for every validator the keymanager API gives no graffiti of its own. At most 32 bytes as UTF-8, right-padded with zeros. Refused rather than truncated if longer. An ethlambda beacon node appends both clients' versions to it (see [`rpc.md`](rpc.md#validator-endpoints)) | | `--enable-keymanager` | off | Serve the keymanager API: keystores, plus each validator's own fee recipient, graffiti and gas limit. Off by default because it mutates key material | +| `--builder-proposals` | off | Propose builder blocks: register each validator with the beacon node's builder (an ethlambda node: `--builder`) and accept a blinded block when the builder's bid wins. 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` | | `--init-slashing-protection` | off | Register every loaded key with no slashing-protection record as never having signed, instead of refusing to start. Dangerous: only for keys that have signed nothing anywhere for at least two epochs, and even then it rules out a double vote only, not a surround of an older vote; a key with a history should bring it instead. Never lowers an existing record; remove it once the keys are registered, and startup warns while it is set for nothing | ### What it does today @@ -314,6 +317,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 @@ -334,7 +348,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, @@ -427,9 +446,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 70de4ac0..10b6ae09 100644 --- a/docs/metrics.md +++ b/docs/metrics.md @@ -748,6 +748,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 | @@ -764,13 +768,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 a42f2b17..24107cfb 100644 --- a/docs/spec_deviations.md +++ b/docs/spec_deviations.md @@ -760,3 +760,40 @@ 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. A blinded block +has the root of the full block it stands for, so slashing protection records +its slot like any other block's, durably and before it is signed: one slot, one +signature, whichever way the node answered. Three departures: + +- **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 + slashing protection 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. The producing node is recorded with the + block's slot and root, and a publish for any other block is refused, as + Teku does; Lighthouse, Lodestar and Nimbus fail over or broadcast instead. + Two status codes are read the wrong way: some of ethlambda's own 500s are + definite (no builder configured, the block failing the node's checks) and + count as unconfirmed, and a proxy's 503 sent after it forwarded the request + counts as definite. Neither leads to a retry or a second signature. +- **A blinded block stays signed but undelivered for longer.** The node waits + up to 5 s on the relay and the client up to 8 s on the node, against well + under a second for a full block. The publish runs on its own task so the + proposal budget cannot cut it off, but the process handles no signals: a + SIGINT in that window ends the task with the request. The record of the + latest blinded block (`signed_blinded_at`) is one per process, not per + validator, since one slot has one proposer. +- **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.