diff --git a/.github/workflows/test-suite.yml b/.github/workflows/test-suite.yml index cc7282c3517..7344a9367b7 100644 --- a/.github/workflows/test-suite.yml +++ b/.github/workflows/test-suite.yml @@ -324,7 +324,7 @@ jobs: channel: stable cache-target: release components: rustfmt,clippy - bins: cargo-audit + bins: cargo-audit,cargo-deny - name: Check formatting with cargo fmt run: make cargo-fmt - name: Lint code for quality and style with Clippy @@ -337,6 +337,8 @@ jobs: run: make arbitrary-fuzz - name: Run cargo audit run: make audit-CI + - name: Run cargo deny + run: make deny-CI - name: Run cargo vendor to make sure dependencies can be vendored for packaging, reproducibility and archival purpose run: CARGO_HOME=$(readlink -f $HOME) make vendor - name: Markdown-linter diff --git a/Makefile b/Makefile index c1cccb92705..4426b941aaa 100644 --- a/Makefile +++ b/Makefile @@ -326,6 +326,15 @@ install-audit: audit-CI: cargo audit +# Runs cargo deny (check for banned crates, duplicate versions, and source restrictions) +deny: install-deny deny-CI + +install-deny: + cargo install --force cargo-deny --version 0.18.2 + +deny-CI: + cargo deny check bans sources --hide-inclusion-graph + # Runs `cargo vendor` to make sure dependencies can be vendored for packaging, reproducibility and archival purpose. vendor: cargo vendor diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 72eb70a2f04..0077eb55642 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -33,7 +33,7 @@ use crate::events::ServerSentEventHandler; use crate::execution_payload::{NotifyExecutionLayer, PreparePayloadHandle, get_execution_payload}; use crate::fetch_blobs::EngineGetBlobsOutput; use crate::fork_choice_signal::{ForkChoiceSignalRx, ForkChoiceSignalTx, ForkChoiceWaitResult}; -use crate::graffiti_calculator::GraffitiCalculator; +use crate::graffiti_calculator::{GraffitiCalculator, GraffitiSettings}; use crate::kzg_utils::reconstruct_blobs; use crate::light_client_finality_update_verification::{ Error as LightClientFinalityUpdateError, VerifiedLightClientFinalityUpdate, @@ -4510,7 +4510,7 @@ impl BeaconChain { self: &Arc, randao_reveal: Signature, slot: Slot, - validator_graffiti: Option, + graffiti_settings: GraffitiSettings, verification: ProduceBlockVerification, builder_boost_factor: Option, block_production_version: BlockProductionVersion, @@ -4544,7 +4544,7 @@ impl BeaconChain { state_root_opt, slot, randao_reveal, - validator_graffiti, + graffiti_settings, verification, builder_boost_factor, block_production_version, @@ -5077,7 +5077,7 @@ impl BeaconChain { state_root_opt: Option, produce_at_slot: Slot, randao_reveal: Signature, - validator_graffiti: Option, + graffiti_settings: GraffitiSettings, verification: ProduceBlockVerification, builder_boost_factor: Option, block_production_version: BlockProductionVersion, @@ -5088,7 +5088,7 @@ impl BeaconChain { let chain = self.clone(); let graffiti = self .graffiti_calculator - .get_graffiti(validator_graffiti) + .get_graffiti(graffiti_settings) .await; let span = Span::current(); let mut partial_beacon_block = self diff --git a/beacon_node/beacon_chain/src/graffiti_calculator.rs b/beacon_node/beacon_chain/src/graffiti_calculator.rs index 56808e0e67e..85470715c9f 100644 --- a/beacon_node/beacon_chain/src/graffiti_calculator.rs +++ b/beacon_node/beacon_chain/src/graffiti_calculator.rs @@ -1,5 +1,6 @@ use crate::BeaconChain; use crate::BeaconChainTypes; +use eth2::types::GraffitiPolicy; use execution_layer::{CommitPrefix, ExecutionLayer, http::ENGINE_GET_CLIENT_VERSION_V1}; use logging::crit; use serde::{Deserialize, Serialize}; @@ -48,6 +49,25 @@ impl Debug for GraffitiOrigin { } } +pub enum GraffitiSettings { + Unspecified, + Specified { + graffiti: Graffiti, + policy: GraffitiPolicy, + }, +} + +impl GraffitiSettings { + pub fn new(validator_graffiti: Option, policy: Option) -> Self { + validator_graffiti + .map(|graffiti| Self::Specified { + graffiti, + policy: policy.unwrap_or(GraffitiPolicy::PreserveUserGraffiti), + }) + .unwrap_or(Self::Unspecified) + } +} + pub struct GraffitiCalculator { pub beacon_graffiti: GraffitiOrigin, execution_layer: Option>, @@ -73,11 +93,19 @@ impl GraffitiCalculator { /// 2. Graffiti specified by the user via beacon node CLI options. /// 3. The EL & CL client version string, applicable when the EL supports version specification. /// 4. The default lighthouse version string, used if the EL lacks version specification support. - pub async fn get_graffiti(&self, validator_graffiti: Option) -> Graffiti { - if let Some(graffiti) = validator_graffiti { - return graffiti; + pub async fn get_graffiti(&self, graffiti_settings: GraffitiSettings) -> Graffiti { + match graffiti_settings { + GraffitiSettings::Specified { graffiti, policy } => match policy { + GraffitiPolicy::PreserveUserGraffiti => graffiti, + GraffitiPolicy::AppendClientVersions => { + self.calculate_combined_graffiti(Some(graffiti)).await + } + }, + GraffitiSettings::Unspecified => self.calculate_combined_graffiti(None).await, } + } + async fn calculate_combined_graffiti(&self, validator_graffiti: Option) -> Graffiti { match self.beacon_graffiti { GraffitiOrigin::UserSpecified(graffiti) => graffiti, GraffitiOrigin::Calculated(default_graffiti) => { @@ -133,7 +161,7 @@ impl GraffitiCalculator { CommitPrefix("00000000".to_string()) }); - engine_version.calculate_graffiti(lighthouse_commit_prefix) + engine_version.calculate_graffiti(lighthouse_commit_prefix, validator_graffiti) } } } @@ -224,8 +252,10 @@ async fn engine_version_cache_refresh_service( #[cfg(test)] mod tests { use crate::ChainConfig; + use crate::graffiti_calculator::GraffitiSettings; use crate::test_utils::{BeaconChainHarness, EphemeralHarnessType, test_spec}; use bls::Keypair; + use eth2::types::GraffitiPolicy; use execution_layer::EngineCapabilities; use execution_layer::test_utils::{DEFAULT_CLIENT_VERSION, DEFAULT_ENGINE_CAPABILITIES}; use std::sync::Arc; @@ -281,8 +311,12 @@ mod tests { let version_bytes = std::cmp::min(lighthouse_version::VERSION.len(), GRAFFITI_BYTES_LEN); // grab the slice of the graffiti that corresponds to the lighthouse version - let graffiti_slice = - &harness.chain.graffiti_calculator.get_graffiti(None).await.0[..version_bytes]; + let graffiti_slice = &harness + .chain + .graffiti_calculator + .get_graffiti(GraffitiSettings::Unspecified) + .await + .0[..version_bytes]; // convert graffiti bytes slice to ascii for easy debugging if this test should fail let graffiti_str = @@ -303,7 +337,12 @@ mod tests { let spec = Arc::new(test_spec::()); let harness = get_harness(VALIDATOR_COUNT, spec, None); - let found_graffiti_bytes = harness.chain.graffiti_calculator.get_graffiti(None).await.0; + let found_graffiti_bytes = harness + .chain + .graffiti_calculator + .get_graffiti(GraffitiSettings::Unspecified) + .await + .0; let mock_commit = DEFAULT_CLIENT_VERSION.commit.clone(); let expected_graffiti_string = format!( @@ -352,7 +391,10 @@ mod tests { let found_graffiti = harness .chain .graffiti_calculator - .get_graffiti(Some(Graffiti::from(graffiti_bytes))) + .get_graffiti(GraffitiSettings::new( + Some(Graffiti::from(graffiti_bytes)), + Some(GraffitiPolicy::PreserveUserGraffiti), + )) .await; assert_eq!( @@ -360,4 +402,98 @@ mod tests { "0x6e6963652067726166666974692062726f000000000000000000000000000000" ); } + + #[tokio::test] + async fn check_append_el_version_graffiti_various_length() { + let spec = Arc::new(test_spec::()); + let harness = get_harness(VALIDATOR_COUNT, spec, None); + + let graffiti_vec = vec![ + // less than 20 characters, example below is 19 characters + "This is my graffiti", + // 20-23 characters, example below is 22 characters + "This is my graffiti yo", + // 24-27 characters, example below is 26 characters + "This is my graffiti string", + // 28-29 characters, example below is 29 characters + "This is my graffiti string yo", + // 30-32 characters, example below is 32 characters + "This is my graffiti string yo yo", + ]; + + for graffiti in graffiti_vec { + let mut graffiti_bytes = [0; GRAFFITI_BYTES_LEN]; + graffiti_bytes[..graffiti.len()].copy_from_slice(graffiti.as_bytes()); + + // To test appending client version info with user specified graffiti + let policy = GraffitiPolicy::AppendClientVersions; + let found_graffiti_bytes = harness + .chain + .graffiti_calculator + .get_graffiti(GraffitiSettings::Specified { + graffiti: Graffiti::from(graffiti_bytes), + policy, + }) + .await + .0; + + let mock_commit = DEFAULT_CLIENT_VERSION.commit.clone(); + + let graffiti_length = graffiti.len(); + let append_graffiti_string = match graffiti_length { + 0..=19 => format!( + "{}{}{}{}", + DEFAULT_CLIENT_VERSION.code, + mock_commit + .strip_prefix("0x") + .unwrap_or("&mock_commit") + .get(0..4) + .expect("should get first 2 bytes in hex"), + "LH", + lighthouse_version::COMMIT_PREFIX + .get(0..4) + .expect("should get first 2 bytes in hex") + ), + 20..=23 => format!( + "{}{}{}{}", + DEFAULT_CLIENT_VERSION.code, + mock_commit + .strip_prefix("0x") + .unwrap_or("&mock_commit") + .get(0..2) + .expect("should get first 2 bytes in hex"), + "LH", + lighthouse_version::COMMIT_PREFIX + .get(0..2) + .expect("should get first 2 bytes in hex") + ), + 24..=27 => format!("{}{}", DEFAULT_CLIENT_VERSION.code, "LH",), + 28..=29 => DEFAULT_CLIENT_VERSION.code.to_string(), + // when user graffiti length is 30-32 characters, append nothing + 30..=32 => String::new(), + _ => panic!( + "graffiti length should be less than or equal to GRAFFITI_BYTES_LEN (32 characters)" + ), + }; + + let expected_graffiti_string = if append_graffiti_string.is_empty() { + // for the case of empty append_graffiti_string, i.e., user-specified graffiti is 30-32 characters + graffiti.to_string() + } else { + // There is a space between the client version info and user graffiti + // as defined in calculate_graffiti function in engine_api.rs + format!("{} {}", append_graffiti_string, graffiti) + }; + + let expected_graffiti_prefix_bytes = expected_graffiti_string.as_bytes(); + let expected_graffiti_prefix_len = + std::cmp::min(expected_graffiti_prefix_bytes.len(), GRAFFITI_BYTES_LEN); + + let found_graffiti_string = + std::str::from_utf8(&found_graffiti_bytes[..expected_graffiti_prefix_len]) + .expect("bytes should convert nicely to ascii"); + + assert_eq!(expected_graffiti_string, found_graffiti_string); + } + } } diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index ba594c1acfb..354c0f23f8d 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -2,6 +2,7 @@ use crate::blob_verification::GossipVerifiedBlob; use crate::block_verification_types::{AsBlock, RpcBlock}; use crate::custody_context::NodeCustodyType; use crate::data_column_verification::CustodyDataColumn; +use crate::graffiti_calculator::GraffitiSettings; use crate::kzg_utils::build_data_column_sidecars; use crate::observed_operations::ObservationOutcome; pub use crate::persisted_beacon_chain::PersistedBeaconChain; @@ -23,7 +24,7 @@ use bls::get_withdrawal_credentials; use bls::{ AggregateSignature, Keypair, PublicKey, PublicKeyBytes, SecretKey, Signature, SignatureBytes, }; -use eth2::types::SignedBlockContentsTuple; +use eth2::types::{GraffitiPolicy, SignedBlockContentsTuple}; use execution_layer::test_utils::generate_genesis_header; use execution_layer::{ ExecutionLayer, @@ -944,6 +945,8 @@ where // BeaconChain errors out with `DuplicateFullyImported`. Vary the graffiti so that we produce // different blocks each time. let graffiti = Graffiti::from(self.rng.lock().random::<[u8; 32]>()); + let graffiti_settings = + GraffitiSettings::new(Some(graffiti), Some(GraffitiPolicy::PreserveUserGraffiti)); let randao_reveal = self.sign_randao_reveal(&state, proposer_index, slot); @@ -957,7 +960,7 @@ where None, slot, randao_reveal, - Some(graffiti), + graffiti_settings, ProduceBlockVerification::VerifyRandao, builder_boost_factor, BlockProductionVersion::V3, @@ -1001,6 +1004,8 @@ where // BeaconChain errors out with `DuplicateFullyImported`. Vary the graffiti so that we produce // different blocks each time. let graffiti = Graffiti::from(self.rng.lock().random::<[u8; 32]>()); + let graffiti_settings = + GraffitiSettings::new(Some(graffiti), Some(GraffitiPolicy::PreserveUserGraffiti)); let randao_reveal = self.sign_randao_reveal(&state, proposer_index, slot); @@ -1011,7 +1016,7 @@ where None, slot, randao_reveal, - Some(graffiti), + graffiti_settings, ProduceBlockVerification::VerifyRandao, None, BlockProductionVersion::FullV2, @@ -1060,6 +1065,8 @@ where // BeaconChain errors out with `DuplicateFullyImported`. Vary the graffiti so that we produce // different blocks each time. let graffiti = Graffiti::from(self.rng.lock().random::<[u8; 32]>()); + let graffiti_settings = + GraffitiSettings::new(Some(graffiti), Some(GraffitiPolicy::PreserveUserGraffiti)); let randao_reveal = self.sign_randao_reveal(&state, proposer_index, slot); @@ -1072,7 +1079,7 @@ where None, slot, randao_reveal, - Some(graffiti), + graffiti_settings, ProduceBlockVerification::VerifyRandao, None, BlockProductionVersion::FullV2, diff --git a/beacon_node/execution_layer/src/engine_api.rs b/beacon_node/execution_layer/src/engine_api.rs index bf913fe5101..32090bccfc9 100644 --- a/beacon_node/execution_layer/src/engine_api.rs +++ b/beacon_node/execution_layer/src/engine_api.rs @@ -735,8 +735,12 @@ pub struct ClientVersionV1 { } impl ClientVersionV1 { - pub fn calculate_graffiti(&self, lighthouse_commit_prefix: CommitPrefix) -> Graffiti { - let graffiti_string = format!( + pub fn calculate_graffiti( + &self, + lighthouse_commit_prefix: CommitPrefix, + validator_graffiti: Option, + ) -> Graffiti { + let append_graffiti_full = format!( "{}{}LH{}", self.code, self.commit @@ -750,6 +754,53 @@ impl ClientVersionV1 { .unwrap_or("0000") .to_lowercase(), ); + + // Implement the special case here: + // https://hackmd.io/@wmoBhF17RAOH2NZ5bNXJVg/BJX2c9gja#SPECIAL-CASE-the-flexible-standard + let append_graffiti_one_byte = format!( + "{}{}LH{}", + self.code, + self.commit + .0 + .get(..2) + .unwrap_or(self.commit.0.as_str()) + .to_lowercase(), + lighthouse_commit_prefix + .0 + .get(..2) + .unwrap_or("00") + .to_lowercase(), + ); + + let append_graffiti_no_commit = format!("{}LH", self.code); + let append_graffiti_only_el = format!("{}", self.code); + + let graffiti_string = if let Some(graffiti) = validator_graffiti { + let graffiti_length = graffiti.as_utf8_lossy().len(); + let graffiti_str = graffiti.as_utf8_lossy(); + + // 12 characters for append_graffiti_full, plus one character for spacing + // that leaves user specified graffiti to be 32-12-1 = 19 characters max, i.e., <20 + if graffiti_length < 20 { + format!("{} {}", append_graffiti_full, graffiti_str) + // user-specified graffiti is between 20-23 characters + } else if (20..24).contains(&graffiti_length) { + format!("{} {}", append_graffiti_one_byte, graffiti_str) + // user-specified graffiti is between 24-27 characters + } else if (24..28).contains(&graffiti_length) { + format!("{} {}", append_graffiti_no_commit, graffiti_str) + // user-specified graffiti is between 28-29 characters + } else if (28..30).contains(&graffiti_length) { + format!("{} {}", append_graffiti_only_el, graffiti_str) + // if user-specified graffiti is between 30-32 characters, append nothing + } else { + return graffiti; + } + } else { + // if no validator_graffiti (user doesn't specify), use the full client version info graffiti + append_graffiti_full + }; + let mut graffiti_bytes = [0u8; GRAFFITI_BYTES_LEN]; let bytes_to_copy = std::cmp::min(graffiti_string.len(), GRAFFITI_BYTES_LEN); graffiti_bytes[..bytes_to_copy] diff --git a/beacon_node/http_api/src/produce_block.rs b/beacon_node/http_api/src/produce_block.rs index 472ec0b65e4..3bd0cec7e33 100644 --- a/beacon_node/http_api/src/produce_block.rs +++ b/beacon_node/http_api/src/produce_block.rs @@ -6,6 +6,7 @@ use crate::{ add_ssz_content_type_header, beacon_response, inconsistent_fork_rejection, }, }; +use beacon_chain::graffiti_calculator::GraffitiSettings; use beacon_chain::{ BeaconBlockResponseWrapper, BeaconChain, BeaconChainTypes, ProduceBlockVerification, }; @@ -68,11 +69,13 @@ pub async fn produce_block_v3( query.builder_boost_factor }; + let graffiti_settings = GraffitiSettings::new(query.graffiti, query.graffiti_policy); + let block_response_type = chain .produce_block_with_verification( randao_reveal, slot, - query.graffiti, + graffiti_settings, randao_verification, builder_boost_factor, BlockProductionVersion::V3, @@ -148,11 +151,13 @@ pub async fn produce_blinded_block_v2( })?; let randao_verification = get_randao_verification(&query, randao_reveal.is_infinity())?; + let graffiti_settings = GraffitiSettings::new(query.graffiti, query.graffiti_policy); + let block_response_type = chain .produce_block_with_verification( randao_reveal, slot, - query.graffiti, + graffiti_settings, randao_verification, None, BlockProductionVersion::BlindedV2, @@ -182,12 +187,13 @@ pub async fn produce_block_v2( })?; let randao_verification = get_randao_verification(&query, randao_reveal.is_infinity())?; + let graffiti_settings = GraffitiSettings::new(query.graffiti, query.graffiti_policy); let block_response_type = chain .produce_block_with_verification( randao_reveal, slot, - query.graffiti, + graffiti_settings, randao_verification, None, BlockProductionVersion::FullV2, diff --git a/beacon_node/http_api/tests/interactive_tests.rs b/beacon_node/http_api/tests/interactive_tests.rs index a00eed87ae8..b04c812773a 100644 --- a/beacon_node/http_api/tests/interactive_tests.rs +++ b/beacon_node/http_api/tests/interactive_tests.rs @@ -646,7 +646,7 @@ pub async fn proposer_boost_re_org_test( .into(); let (unsigned_block_type, _) = tester .client - .get_validator_blocks_v3::(slot_c, &randao_reveal, None, None) + .get_validator_blocks_v3::(slot_c, &randao_reveal, None, None, None) .await .unwrap(); diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index f8eba0ee2b7..ed7abead18a 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -3681,7 +3681,7 @@ impl ApiTester { let (response, metadata) = self .client - .get_validator_blocks_v3_ssz::(slot, &randao_reveal, None, None) + .get_validator_blocks_v3_ssz::(slot, &randao_reveal, None, None, None) .await .unwrap(); @@ -4646,7 +4646,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -4673,7 +4673,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None, Some(0)) + .get_validator_blocks_v3::(slot, &randao_reveal, None, Some(0), None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -4701,7 +4701,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None, Some(u64::MAX)) + .get_validator_blocks_v3::(slot, &randao_reveal, None, Some(u64::MAX), None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -4858,7 +4858,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -4939,7 +4939,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -5034,7 +5034,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -5125,7 +5125,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -5216,7 +5216,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -5305,7 +5305,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -5366,7 +5366,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -5437,7 +5437,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -5552,7 +5552,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(next_slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(next_slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -5573,7 +5573,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(next_slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(next_slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -5708,7 +5708,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(next_slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(next_slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -5739,7 +5739,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(next_slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(next_slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -5821,7 +5821,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -5895,7 +5895,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -5964,7 +5964,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -6033,7 +6033,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -6100,7 +6100,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -6174,7 +6174,7 @@ impl ApiTester { let (payload_type, metadata) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None, None) .await .unwrap(); Self::check_block_v3_metadata(&metadata, &payload_type); @@ -6864,6 +6864,82 @@ impl ApiTester { } self } + + async fn get_validator_blocks_v3_path_graffiti_policy(self) -> Self { + let slot = self.chain.slot().unwrap(); + let epoch = self.chain.epoch().unwrap(); + let (_, randao_reveal) = self.get_test_randao(slot, epoch).await; + let graffiti = Some(Graffiti::from([0; GRAFFITI_BYTES_LEN])); + let builder_boost_factor = None; + + // Default case where GraffitiPolicy is None + let default_path = self + .client + .get_validator_blocks_v3_path( + slot, + &randao_reveal, + graffiti.as_ref(), + SkipRandaoVerification::Yes, + builder_boost_factor, + None, + ) + .await + .unwrap(); + + let query_default_path = default_path.query().unwrap_or(""); + // When GraffitiPolicy is None, the HTTP API query path should not contain "graffiti_policy" + assert!( + !query_default_path.contains("graffiti_policy"), + "URL should not contain graffiti_policy parameter (same as PreserveUserGraffiti). URL is: {}", + query_default_path + ); + + let preserve_path = self + .client + .get_validator_blocks_v3_path( + slot, + &randao_reveal, + graffiti.as_ref(), + SkipRandaoVerification::Yes, + builder_boost_factor, + Some(GraffitiPolicy::PreserveUserGraffiti), + ) + .await + .unwrap(); + + let query_preserve_path = preserve_path.query().unwrap_or(""); + // When GraffitiPolicy is set to PreserveUserGraffiti, the HTTP API query path should not contain "graffiti_policy" + assert!( + !query_preserve_path.contains("graffiti_policy"), + "URL should not contain graffiti_policy parameter when using PreserveUserGraffiti. URL is: {}", + query_preserve_path + ); + + // The HTTP API query path for PreserveUserGraffiti should be the same as the default + assert_eq!(query_default_path, query_preserve_path); + + let append_path = self + .client + .get_validator_blocks_v3_path( + slot, + &randao_reveal, + graffiti.as_ref(), + SkipRandaoVerification::No, + builder_boost_factor, + Some(GraffitiPolicy::AppendClientVersions), + ) + .await + .unwrap(); + + let query_append_path = append_path.query().unwrap_or(""); + // When GraffitiPolicy is AppendClientVersions, the HTTP API query path should contain "graffiti_policy" + assert!( + query_append_path.contains("graffiti_policy"), + "URL should contain graffiti_policy=AppendClientVersions parameter. URL is: {}", + query_append_path + ); + self + } } async fn poll_events, eth2::Error>> + Unpin, E: EthSpec>( @@ -8054,3 +8130,11 @@ async fn get_beacon_rewards_blocks_electra() { .test_beacon_block_rewards_electra() .await; } + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn get_validator_blocks_v3_http_api_path() { + ApiTester::new() + .await + .get_validator_blocks_v3_path_graffiti_policy() + .await; +} diff --git a/beacon_node/store/src/chunked_iter.rs b/beacon_node/store/src/chunked_iter.rs deleted file mode 100644 index 72e5d9c7af0..00000000000 --- a/beacon_node/store/src/chunked_iter.rs +++ /dev/null @@ -1,120 +0,0 @@ -use crate::chunked_vector::{Chunk, Field, chunk_key}; -use crate::{HotColdDB, ItemStore}; -use tracing::error; -use types::{ChainSpec, EthSpec, Slot}; - -/// Iterator over the values of a `BeaconState` vector field (like `block_roots`). -/// -/// Uses the freezer DB's separate table to load the values. -pub struct ChunkedVectorIter<'a, F, E, Hot, Cold> -where - F: Field, - E: EthSpec, - Hot: ItemStore, - Cold: ItemStore, -{ - pub(crate) store: &'a HotColdDB, - current_vindex: usize, - pub(crate) end_vindex: usize, - next_cindex: usize, - current_chunk: Chunk, -} - -impl<'a, F, E, Hot, Cold> ChunkedVectorIter<'a, F, E, Hot, Cold> -where - F: Field, - E: EthSpec, - Hot: ItemStore, - Cold: ItemStore, -{ - /// Create a new iterator which can yield elements from `start_vindex` up to the last - /// index stored by the restore point at `last_restore_point_slot`. - /// - /// The `freezer_upper_limit` slot should be the slot of a recent restore point as obtained from - /// `Root::freezer_upper_limit`. We pass it as a parameter so that the caller can - /// maintain a stable view of the database (see `HybridForwardsBlockRootsIterator`). - pub fn new( - store: &'a HotColdDB, - start_vindex: usize, - freezer_upper_limit: Slot, - spec: &ChainSpec, - ) -> Self { - let (_, end_vindex) = F::start_and_end_vindex(freezer_upper_limit, spec); - - // Set the next chunk to the one containing `start_vindex`. - let next_cindex = start_vindex / F::chunk_size(); - // Set the current chunk to the empty chunk, it will never be read. - let current_chunk = Chunk::default(); - - Self { - store, - current_vindex: start_vindex, - end_vindex, - next_cindex, - current_chunk, - } - } -} - -impl Iterator for ChunkedVectorIter<'_, F, E, Hot, Cold> -where - F: Field, - E: EthSpec, - Hot: ItemStore, - Cold: ItemStore, -{ - type Item = (usize, F::Value); - - fn next(&mut self) -> Option { - let chunk_size = F::chunk_size(); - - // Range exhausted, return `None` forever. - if self.current_vindex >= self.end_vindex { - None - } - // Value lies in the current chunk, return it. - else if self.current_vindex < self.next_cindex * chunk_size { - let vindex = self.current_vindex; - let val = self - .current_chunk - .values - .get(vindex % chunk_size) - .cloned() - .or_else(|| { - error!( - vector_index = vindex, - "Missing chunk value in forwards iterator" - ); - None - })?; - self.current_vindex += 1; - Some((vindex, val)) - } - // Need to load the next chunk, load it and recurse back into the in-range case. - else { - self.current_chunk = Chunk::load( - &self.store.cold_db, - F::column(), - &chunk_key(self.next_cindex), - ) - .map_err(|e| { - error!( - chunk_index = self.next_cindex, - error = ?e, - "Database error in forwards iterator" - ); - e - }) - .ok()? - .or_else(|| { - error!( - chunk_index = self.next_cindex, - "Missing chunk in forwards iterator" - ); - None - })?; - self.next_cindex += 1; - self.next() - } - } -} diff --git a/beacon_node/store/src/chunked_vector.rs b/beacon_node/store/src/chunked_vector.rs deleted file mode 100644 index 9c8114e0c14..00000000000 --- a/beacon_node/store/src/chunked_vector.rs +++ /dev/null @@ -1,922 +0,0 @@ -//! Space-efficient storage for `BeaconState` vector fields. -//! -//! This module provides logic for splitting the `Vector` fields of a `BeaconState` into -//! chunks, and storing those chunks in contiguous ranges in the on-disk database. The motiviation -//! for doing this is avoiding massive duplication in every on-disk state. For example, rather than -//! storing the whole `historical_roots` vector, which is updated once every couple of thousand -//! slots, at every slot, we instead store all the historical values as a chunked vector on-disk, -//! and fetch only the slice we need when reconstructing the `historical_roots` of a state. -//! -//! ## Terminology -//! -//! * **Chunk size**: the number of vector values stored per on-disk chunk. -//! * **Vector index** (vindex): index into all the historical values, identifying a single element -//! of the vector being stored. -//! * **Chunk index** (cindex): index into the keyspace of the on-disk database, identifying a chunk -//! of elements. To find the chunk index of a vector index: `cindex = vindex / chunk_size`. -use self::UpdatePattern::*; -use crate::*; -use milhouse::{List, Vector}; -use ssz::{Decode, Encode}; -use typenum::Unsigned; -use types::historical_summary::HistoricalSummary; - -/// Description of how a `BeaconState` field is updated during state processing. -/// -/// When storing a state, this allows us to efficiently store only those entries -/// which are not present in the DB already. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum UpdatePattern { - /// The value is updated once per `n` slots. - OncePerNSlots { - n: u64, - /// The slot at which the field begins to accumulate values. - /// - /// The field should not be read or written until `activation_slot` is reached, and the - /// activation slot should act as an offset when converting slots to vector indices. - activation_slot: Option, - /// The slot at which the field ceases to accumulate values. - /// - /// If this is `None` then the field is continually updated. - deactivation_slot: Option, - }, - /// The value is updated once per epoch, for the epoch `current_epoch - lag`. - OncePerEpoch { lag: u64 }, -} - -/// Map a chunk index to bytes that can be used to key the NoSQL database. -/// -/// We shift chunks up by 1 to make room for a genesis chunk that is handled separately. -pub fn chunk_key(cindex: usize) -> [u8; 8] { - (cindex as u64 + 1).to_be_bytes() -} - -/// Return the database key for the genesis value. -fn genesis_value_key() -> [u8; 8] { - 0u64.to_be_bytes() -} - -/// Trait for types representing fields of the `BeaconState`. -/// -/// All of the required methods are type-level, because we do most things with fields at the -/// type-level. We require their value-level witnesses to be `Copy` so that we can avoid the -/// turbofish when calling functions like `store_updated_vector`. -pub trait Field: Copy { - /// The type of value stored in this field: the `T` from `Vector`. - /// - /// The `Default` impl will be used to fill extra vector entries. - type Value: Default + std::fmt::Debug + milhouse::Value; - // Decode + Encode + Default + Clone + PartialEq + std::fmt::Debug - - /// The length of this field: the `N` from `Vector`. - type Length: Unsigned; - - /// The database column where the integer-indexed chunks for this field should be stored. - /// - /// Each field's column **must** be unique. - fn column() -> DBColumn; - - /// Update pattern for this field, so that we can do differential updates. - fn update_pattern(spec: &ChainSpec) -> UpdatePattern; - - /// The number of values to store per chunk on disk. - /// - /// Default is 128 so that we read/write 4K pages when the values are 32 bytes. - // TODO: benchmark and optimise this parameter - fn chunk_size() -> usize { - 128 - } - - /// Convert a v-index (vector index) to a chunk index. - fn chunk_index(vindex: usize) -> usize { - vindex / Self::chunk_size() - } - - /// Get the value of this field at the given vector index, from the state. - fn get_value( - state: &BeaconState, - vindex: u64, - spec: &ChainSpec, - ) -> Result; - - /// True if this is a `FixedLengthField`, false otherwise. - fn is_fixed_length() -> bool; - - /// Compute the start and end vector indices of the slice of history required at `current_slot`. - /// - /// ## Example - /// - /// If we have a field that is updated once per epoch, then the end vindex will be - /// `current_epoch + 1`, because we want to include the value for the current epoch, and the - /// start vindex will be `end_vindex - Self::Length`, because that's how far back we can look. - fn start_and_end_vindex(current_slot: Slot, spec: &ChainSpec) -> (usize, usize) { - // We take advantage of saturating subtraction on slots and epochs - match Self::update_pattern(spec) { - OncePerNSlots { - n, - activation_slot, - deactivation_slot, - } => { - // Per-slot changes exclude the index for the current slot, because - // it won't be set until the slot completes (think of `state_roots`, `block_roots`). - // This also works for the `historical_roots` because at the `n`th slot, the 0th - // entry of the list is created, and before that the list is empty. - // - // To account for the switch from historical roots to historical summaries at - // Capella we also modify the current slot by the activation and deactivation slots. - // The activation slot acts as an offset (subtraction) while the deactivation slot - // acts as a clamp (min). - let slot_with_clamp = deactivation_slot.map_or(current_slot, |deactivation_slot| { - std::cmp::min(current_slot, deactivation_slot) - }); - let slot_with_clamp_and_offset = if let Some(activation_slot) = activation_slot { - slot_with_clamp - activation_slot - } else { - // Return (0, 0) to indicate that the field should not be read/written. - return (0, 0); - }; - let end_vindex = slot_with_clamp_and_offset / n; - let start_vindex = end_vindex - Self::Length::to_u64(); - (start_vindex.as_usize(), end_vindex.as_usize()) - } - OncePerEpoch { lag } => { - // Per-epoch changes include the index for the current epoch, because it - // will have been set at the most recent epoch boundary. - let current_epoch = current_slot.epoch(E::slots_per_epoch()); - let end_epoch = current_epoch + 1 - lag; - let start_epoch = end_epoch + lag - Self::Length::to_u64(); - (start_epoch.as_usize(), end_epoch.as_usize()) - } - } - } - - /// Given an `existing_chunk` stored in the DB, construct an updated chunk to replace it. - fn get_updated_chunk( - existing_chunk: &Chunk, - chunk_index: usize, - start_vindex: usize, - end_vindex: usize, - state: &BeaconState, - spec: &ChainSpec, - ) -> Result, Error> { - let chunk_size = Self::chunk_size(); - let mut new_chunk = Chunk::new(vec![Self::Value::default(); chunk_size]); - - for i in 0..chunk_size { - let vindex = chunk_index * chunk_size + i; - if vindex >= start_vindex && vindex < end_vindex { - let vector_value = Self::get_value(state, vindex as u64, spec)?; - - if let Some(existing_value) = existing_chunk.values.get(i) - && *existing_value != vector_value - && *existing_value != Self::Value::default() - { - return Err(ChunkError::Inconsistent { - field: Self::column(), - chunk_index, - existing_value: format!("{:?}", existing_value), - new_value: format!("{:?}", vector_value), - } - .into()); - } - - new_chunk.values[i] = vector_value; - } else { - new_chunk.values[i] = existing_chunk.values.get(i).cloned().unwrap_or_default(); - } - } - - Ok(new_chunk) - } - - /// Determine whether a state at `slot` possesses (or requires) the genesis value. - fn slot_needs_genesis_value(slot: Slot, spec: &ChainSpec) -> bool { - let (_, end_vindex) = Self::start_and_end_vindex(slot, spec); - match Self::update_pattern(spec) { - // If the end_vindex is less than the length of the vector, then the vector - // has not yet been completely filled with non-genesis values, and so the genesis - // value is still required. - OncePerNSlots { .. } => { - Self::is_fixed_length() && end_vindex < Self::Length::to_usize() - } - // If the field has lag, then it takes an extra `lag` vindices beyond the - // `end_vindex` before the vector has been filled with non-genesis values. - OncePerEpoch { lag } => { - Self::is_fixed_length() && end_vindex + (lag as usize) < Self::Length::to_usize() - } - } - } - - /// Load the genesis value for a fixed length field from the store. - /// - /// This genesis value should be used to fill the initial state of the vector. - fn load_genesis_value>(store: &S) -> Result { - let key = &genesis_value_key()[..]; - let chunk = - Chunk::load(store, Self::column(), key)?.ok_or(ChunkError::MissingGenesisValue)?; - chunk - .values - .first() - .cloned() - .ok_or_else(|| ChunkError::MissingGenesisValue.into()) - } - - /// Store the given `value` as the genesis value for this field, unless stored already. - /// - /// Check the existing value (if any) for consistency with the value we intend to store, and - /// return an error if they are inconsistent. - fn check_and_store_genesis_value>( - store: &S, - value: Self::Value, - ops: &mut Vec, - ) -> Result<(), Error> { - let key = &genesis_value_key()[..]; - - if let Some(existing_chunk) = Chunk::::load(store, Self::column(), key)? { - if existing_chunk.values.len() != 1 { - Err(ChunkError::InvalidGenesisChunk { - field: Self::column(), - expected_len: 1, - observed_len: existing_chunk.values.len(), - } - .into()) - } else if existing_chunk.values[0] != value { - Err(ChunkError::InconsistentGenesisValue { - field: Self::column(), - existing_value: format!("{:?}", existing_chunk.values[0]), - new_value: format!("{:?}", value), - } - .into()) - } else { - Ok(()) - } - } else { - let chunk = Chunk::new(vec![value]); - chunk.store(Self::column(), &genesis_value_key()[..], ops)?; - Ok(()) - } - } - - /// Extract the genesis value for a fixed length field from an - /// - /// Will only return a correct value if `slot_needs_genesis_value(state.slot(), spec) == true`. - fn extract_genesis_value( - state: &BeaconState, - spec: &ChainSpec, - ) -> Result { - let (_, end_vindex) = Self::start_and_end_vindex(state.slot(), spec); - match Self::update_pattern(spec) { - // Genesis value is guaranteed to exist at `end_vindex`, as it won't yet have been - // updated - OncePerNSlots { .. } => Ok(Self::get_value(state, end_vindex as u64, spec)?), - // If there's lag, the value of the field at the vindex *without the lag* - // should still be set to the genesis value. - OncePerEpoch { lag } => Ok(Self::get_value(state, end_vindex as u64 + lag, spec)?), - } - } -} - -/// Marker trait for fixed-length fields (`Vector`). -pub trait FixedLengthField: Field {} - -/// Marker trait for variable-length fields (`List`). -pub trait VariableLengthField: Field {} - -/// Macro to implement the `Field` trait on a new unit struct type. -macro_rules! field { - ($struct_name:ident, $marker_trait:ident, $value_ty:ty, $length_ty:ty, $column:expr, - $update_pattern:expr, $get_value:expr) => { - #[derive(Clone, Copy)] - pub struct $struct_name; - - impl Field for $struct_name - where - E: EthSpec, - { - type Value = $value_ty; - type Length = $length_ty; - - fn column() -> DBColumn { - $column - } - - fn update_pattern(spec: &ChainSpec) -> UpdatePattern { - let update_pattern = $update_pattern; - update_pattern(spec) - } - - fn get_value( - state: &BeaconState, - vindex: u64, - spec: &ChainSpec, - ) -> Result { - let get_value = $get_value; - get_value(state, vindex, spec) - } - - fn is_fixed_length() -> bool { - stringify!($marker_trait) == "FixedLengthField" - } - } - - impl $marker_trait for $struct_name {} - }; -} - -field!( - BlockRootsChunked, - FixedLengthField, - Hash256, - E::SlotsPerHistoricalRoot, - DBColumn::BeaconBlockRootsChunked, - |_| OncePerNSlots { - n: 1, - activation_slot: Some(Slot::new(0)), - deactivation_slot: None - }, - |state: &BeaconState<_>, index, _| safe_modulo_vector_index(state.block_roots(), index) -); - -field!( - StateRootsChunked, - FixedLengthField, - Hash256, - E::SlotsPerHistoricalRoot, - DBColumn::BeaconStateRootsChunked, - |_| OncePerNSlots { - n: 1, - activation_slot: Some(Slot::new(0)), - deactivation_slot: None, - }, - |state: &BeaconState<_>, index, _| safe_modulo_vector_index(state.state_roots(), index) -); - -field!( - HistoricalRoots, - VariableLengthField, - Hash256, - E::HistoricalRootsLimit, - DBColumn::BeaconHistoricalRoots, - |spec: &ChainSpec| OncePerNSlots { - n: E::SlotsPerHistoricalRoot::to_u64(), - activation_slot: Some(Slot::new(0)), - deactivation_slot: spec - .capella_fork_epoch - .map(|fork_epoch| fork_epoch.start_slot(E::slots_per_epoch())), - }, - |state: &BeaconState<_>, index, _| safe_modulo_list_index(state.historical_roots(), index) -); - -field!( - RandaoMixes, - FixedLengthField, - Hash256, - E::EpochsPerHistoricalVector, - DBColumn::BeaconRandaoMixes, - |_| OncePerEpoch { lag: 1 }, - |state: &BeaconState<_>, index, _| safe_modulo_vector_index(state.randao_mixes(), index) -); - -field!( - HistoricalSummaries, - VariableLengthField, - HistoricalSummary, - E::HistoricalRootsLimit, - DBColumn::BeaconHistoricalSummaries, - |spec: &ChainSpec| OncePerNSlots { - n: E::SlotsPerHistoricalRoot::to_u64(), - activation_slot: spec - .capella_fork_epoch - .map(|fork_epoch| fork_epoch.start_slot(E::slots_per_epoch())), - deactivation_slot: None, - }, - |state: &BeaconState<_>, index, _| safe_modulo_list_index( - state - .historical_summaries() - .map_err(|_| ChunkError::InvalidFork)?, - index - ) -); - -pub fn store_updated_vector, E: EthSpec, S: KeyValueStore>( - field: F, - store: &S, - state: &BeaconState, - spec: &ChainSpec, - ops: &mut Vec, -) -> Result<(), Error> { - let chunk_size = F::chunk_size(); - let (start_vindex, end_vindex) = F::start_and_end_vindex(state.slot(), spec); - let start_cindex = start_vindex / chunk_size; - let end_cindex = end_vindex / chunk_size; - - // Store the genesis value if we have access to it, and it hasn't been stored already. - if F::slot_needs_genesis_value(state.slot(), spec) { - let genesis_value = F::extract_genesis_value(state, spec)?; - F::check_and_store_genesis_value(store, genesis_value, ops)?; - } - - // Start by iterating backwards from the last chunk, storing new chunks in the database. - // Stop once a chunk in the database matches what we were about to store, this indicates - // that a previously stored state has already filled-in a portion of the indices covered. - let full_range_checked = store_range( - field, - (start_cindex..=end_cindex).rev(), - start_vindex, - end_vindex, - store, - state, - spec, - ops, - )?; - - // If the previous `store_range` did not check the entire range, it may be the case that the - // state's vector includes elements at low vector indices that are not yet stored in the - // database, so run another `store_range` to ensure these values are also stored. - if !full_range_checked { - store_range( - field, - start_cindex..end_cindex, - start_vindex, - end_vindex, - store, - state, - spec, - ops, - )?; - } - - Ok(()) -} - -#[allow(clippy::too_many_arguments)] -fn store_range( - _: F, - range: I, - start_vindex: usize, - end_vindex: usize, - store: &S, - state: &BeaconState, - spec: &ChainSpec, - ops: &mut Vec, -) -> Result -where - F: Field, - E: EthSpec, - S: KeyValueStore, - I: Iterator, -{ - for chunk_index in range { - let chunk_key = &chunk_key(chunk_index)[..]; - - let existing_chunk = - Chunk::::load(store, F::column(), chunk_key)?.unwrap_or_default(); - - let new_chunk = F::get_updated_chunk( - &existing_chunk, - chunk_index, - start_vindex, - end_vindex, - state, - spec, - )?; - - if new_chunk == existing_chunk { - return Ok(false); - } - - new_chunk.store(F::column(), chunk_key, ops)?; - } - - Ok(true) -} - -// Chunks at the end index are included. -// TODO: could be more efficient with a real range query (perhaps RocksDB) -fn range_query, E: EthSpec, T: Decode + Encode>( - store: &S, - column: DBColumn, - start_index: usize, - end_index: usize, -) -> Result>, Error> { - let range = start_index..=end_index; - let len = range - .end() - // Add one to account for inclusive range. - .saturating_add(1) - .saturating_sub(*range.start()); - let mut result = Vec::with_capacity(len); - - for chunk_index in range { - let key = &chunk_key(chunk_index)[..]; - let chunk = Chunk::load(store, column, key)?.ok_or(ChunkError::Missing { chunk_index })?; - result.push(chunk); - } - - Ok(result) -} - -/// Combine chunks to form a list or vector of all values with vindex in `start_vindex..end_vindex`. -/// -/// The `length` parameter is the length of the vec to construct, with entries set to `default` if -/// they lie outside the vindex range. -fn stitch( - chunks: Vec>, - start_vindex: usize, - end_vindex: usize, - chunk_size: usize, - length: usize, - default: T, -) -> Result, ChunkError> { - if start_vindex + length < end_vindex { - return Err(ChunkError::OversizedRange { - start_vindex, - end_vindex, - length, - }); - } - - let start_cindex = start_vindex / chunk_size; - let end_cindex = end_vindex / chunk_size; - - let mut result = vec![default; length]; - - for (chunk_index, chunk) in (start_cindex..=end_cindex).zip(chunks.into_iter()) { - // All chunks but the last chunk must be full-sized - if chunk_index != end_cindex && chunk.values.len() != chunk_size { - return Err(ChunkError::InvalidSize { - chunk_index, - expected: chunk_size, - actual: chunk.values.len(), - }); - } - - // Copy the chunk entries into the result vector - for (i, value) in chunk.values.into_iter().enumerate() { - let vindex = chunk_index * chunk_size + i; - - if vindex >= start_vindex && vindex < end_vindex { - result[vindex % length] = value; - } - } - } - - Ok(result) -} - -pub fn load_vector_from_db, E: EthSpec, S: KeyValueStore>( - store: &S, - slot: Slot, - spec: &ChainSpec, -) -> Result, Error> { - // Do a range query - let chunk_size = F::chunk_size(); - let (start_vindex, end_vindex) = F::start_and_end_vindex(slot, spec); - let start_cindex = start_vindex / chunk_size; - let end_cindex = end_vindex / chunk_size; - - let chunks = range_query(store, F::column(), start_cindex, end_cindex)?; - - let default = if F::slot_needs_genesis_value(slot, spec) { - F::load_genesis_value(store)? - } else { - F::Value::default() - }; - - let result = stitch( - chunks, - start_vindex, - end_vindex, - chunk_size, - F::Length::to_usize(), - default, - )?; - - Ok(Vector::new(result).map_err(ChunkError::Milhouse)?) -} - -/// The historical roots are stored in vector chunks, despite not actually being a vector. -pub fn load_variable_list_from_db, E: EthSpec, S: KeyValueStore>( - store: &S, - slot: Slot, - spec: &ChainSpec, -) -> Result, Error> { - let chunk_size = F::chunk_size(); - let (start_vindex, end_vindex) = F::start_and_end_vindex(slot, spec); - let start_cindex = start_vindex / chunk_size; - let end_cindex = end_vindex / chunk_size; - - let chunks: Vec> = range_query(store, F::column(), start_cindex, end_cindex)?; - - let mut result = Vec::with_capacity(chunk_size * chunks.len()); - - for (chunk_index, chunk) in chunks.into_iter().enumerate() { - for (i, value) in chunk.values.into_iter().enumerate() { - let vindex = chunk_index * chunk_size + i; - - if vindex >= start_vindex && vindex < end_vindex { - result.push(value); - } - } - } - - Ok(List::new(result).map_err(ChunkError::Milhouse)?) -} - -/// Index into a `List` field of the state, avoiding out of bounds and division by 0. -fn safe_modulo_list_index( - values: &List, - index: u64, -) -> Result { - if values.is_empty() { - Err(ChunkError::ZeroLengthList) - } else { - values - .get(index as usize % values.len()) - .copied() - .ok_or(ChunkError::IndexOutOfBounds { index }) - } -} - -fn safe_modulo_vector_index( - values: &Vector, - index: u64, -) -> Result { - if values.is_empty() { - Err(ChunkError::ZeroLengthVector) - } else { - values - .get(index as usize % values.len()) - .copied() - .ok_or(ChunkError::IndexOutOfBounds { index }) - } -} - -/// A chunk of a fixed-size vector from the `BeaconState`, stored in the database. -#[derive(Debug, Clone, PartialEq)] -pub struct Chunk { - /// A vector of up-to `chunk_size` values. - pub values: Vec, -} - -impl Default for Chunk -where - T: Decode + Encode, -{ - fn default() -> Self { - Chunk { values: vec![] } - } -} - -impl Chunk -where - T: Decode + Encode, -{ - pub fn new(values: Vec) -> Self { - Chunk { values } - } - - pub fn load, E: EthSpec>( - store: &S, - column: DBColumn, - key: &[u8], - ) -> Result, Error> { - store - .get_bytes(column, key)? - .map(|bytes| Self::decode(&bytes)) - .transpose() - } - - pub fn store( - &self, - column: DBColumn, - key: &[u8], - ops: &mut Vec, - ) -> Result<(), Error> { - ops.push(KeyValueStoreOp::PutKeyValue( - column, - key.to_vec(), - self.encode()?, - )); - Ok(()) - } - - /// Attempt to decode a single chunk. - pub fn decode(bytes: &[u8]) -> Result { - if !::is_ssz_fixed_len() { - return Err(Error::from(ChunkError::InvalidType)); - } - - let value_size = ::ssz_fixed_len(); - - if value_size == 0 { - return Err(Error::from(ChunkError::InvalidType)); - } - - let values = bytes - .chunks(value_size) - .map(T::from_ssz_bytes) - .collect::>()?; - - Ok(Chunk { values }) - } - - pub fn encoded_size(&self) -> usize { - self.values.len() * ::ssz_fixed_len() - } - - /// Encode a single chunk as bytes. - pub fn encode(&self) -> Result, Error> { - if !::is_ssz_fixed_len() { - return Err(Error::from(ChunkError::InvalidType)); - } - - Ok(self.values.iter().flat_map(T::as_ssz_bytes).collect()) - } -} - -#[derive(Debug, PartialEq)] -pub enum ChunkError { - ZeroLengthVector, - ZeroLengthList, - IndexOutOfBounds { - index: u64, - }, - InvalidSize { - chunk_index: usize, - expected: usize, - actual: usize, - }, - Missing { - chunk_index: usize, - }, - MissingGenesisValue, - Inconsistent { - field: DBColumn, - chunk_index: usize, - existing_value: String, - new_value: String, - }, - InconsistentGenesisValue { - field: DBColumn, - existing_value: String, - new_value: String, - }, - InvalidGenesisChunk { - field: DBColumn, - expected_len: usize, - observed_len: usize, - }, - InvalidType, - OversizedRange { - start_vindex: usize, - end_vindex: usize, - length: usize, - }, - InvalidFork, - Milhouse(milhouse::Error), -} - -impl From for ChunkError { - fn from(e: milhouse::Error) -> ChunkError { - Self::Milhouse(e) - } -} - -#[cfg(test)] -mod test { - use super::*; - use fixed_bytes::FixedBytesExtended; - use types::MainnetEthSpec as TestSpec; - use types::*; - - fn v(i: u64) -> Hash256 { - Hash256::from_low_u64_be(i) - } - - #[test] - fn stitch_default() { - let chunk_size = 4; - - let chunks = vec![ - Chunk::new(vec![0u64, 1, 2, 3]), - Chunk::new(vec![4, 5, 0, 0]), - ]; - - assert_eq!( - stitch(chunks, 2, 6, chunk_size, 12, 99).unwrap(), - vec![99, 99, 2, 3, 4, 5, 99, 99, 99, 99, 99, 99] - ); - } - - #[test] - fn stitch_basic() { - let chunk_size = 4; - let default = v(0); - - let chunks = vec![ - Chunk::new(vec![v(0), v(1), v(2), v(3)]), - Chunk::new(vec![v(4), v(5), v(6), v(7)]), - Chunk::new(vec![v(8), v(9), v(10), v(11)]), - ]; - - assert_eq!( - stitch(chunks.clone(), 0, 12, chunk_size, 12, default).unwrap(), - (0..12).map(v).collect::>() - ); - - assert_eq!( - stitch(chunks, 2, 10, chunk_size, 8, default).unwrap(), - vec![v(8), v(9), v(2), v(3), v(4), v(5), v(6), v(7)] - ); - } - - #[test] - fn stitch_oversized_range() { - let chunk_size = 4; - let default = 0; - - let chunks = vec![Chunk::new(vec![20u64, 21, 22, 23])]; - - // Args (start_vindex, end_vindex, length) - let args = vec![(0, 21, 20), (0, 2048, 1024), (0, 2, 1)]; - - for (start_vindex, end_vindex, length) in args { - assert_eq!( - stitch( - chunks.clone(), - start_vindex, - end_vindex, - chunk_size, - length, - default - ), - Err(ChunkError::OversizedRange { - start_vindex, - end_vindex, - length, - }) - ); - } - } - - #[test] - fn fixed_length_fields() { - fn test_fixed_length>(_: F, expected: bool) { - assert_eq!(F::is_fixed_length(), expected); - } - test_fixed_length(BlockRootsChunked, true); - test_fixed_length(StateRootsChunked, true); - test_fixed_length(HistoricalRoots, false); - test_fixed_length(RandaoMixes, true); - } - - fn needs_genesis_value_once_per_slot>(_: F) { - let spec = &TestSpec::default_spec(); - let max = F::Length::to_u64(); - for i in 0..max { - assert!( - F::slot_needs_genesis_value(Slot::new(i), spec), - "slot {}", - i - ); - } - assert!(!F::slot_needs_genesis_value(Slot::new(max), spec)); - } - - #[test] - fn needs_genesis_value_block_roots() { - needs_genesis_value_once_per_slot(BlockRootsChunked); - } - - #[test] - fn needs_genesis_value_state_roots() { - needs_genesis_value_once_per_slot(StateRootsChunked); - } - - #[test] - fn needs_genesis_value_historical_roots() { - let spec = &TestSpec::default_spec(); - assert!( - !>::slot_needs_genesis_value(Slot::new(0), spec) - ); - } - - fn needs_genesis_value_test_randao>(_: F) { - let spec = &TestSpec::default_spec(); - let max = TestSpec::slots_per_epoch() * (F::Length::to_u64() - 1); - for i in 0..max { - assert!( - F::slot_needs_genesis_value(Slot::new(i), spec), - "slot {}", - i - ); - } - assert!(!F::slot_needs_genesis_value(Slot::new(max), spec)); - } - - #[test] - fn needs_genesis_value_randao() { - needs_genesis_value_test_randao(RandaoMixes); - } -} diff --git a/beacon_node/store/src/errors.rs b/beacon_node/store/src/errors.rs index 6da99b7bd63..a07cc838863 100644 --- a/beacon_node/store/src/errors.rs +++ b/beacon_node/store/src/errors.rs @@ -1,4 +1,3 @@ -use crate::chunked_vector::ChunkError; use crate::config::StoreConfigError; use crate::hot_cold_store::{HotColdDBError, StateSummaryIteratorError}; use crate::{DBColumn, hdiff}; @@ -13,9 +12,7 @@ pub type Result = std::result::Result; #[derive(Debug)] pub enum Error { SszDecodeError(DecodeError), - VectorChunkError(ChunkError), BeaconStateError(BeaconStateError), - PartialBeaconStateError, HotColdDBError(HotColdDBError), DBError { message: String, @@ -126,12 +123,6 @@ impl From for Error { } } -impl From for Error { - fn from(e: ChunkError) -> Error { - Error::VectorChunkError(e) - } -} - impl From for Error { fn from(e: HotColdDBError) -> Error { Error::HotColdDBError(e) diff --git a/beacon_node/store/src/lib.rs b/beacon_node/store/src/lib.rs index a3d4e4a8cea..ae5b2e1e571 100644 --- a/beacon_node/store/src/lib.rs +++ b/beacon_node/store/src/lib.rs @@ -8,8 +8,6 @@ //! Provides a simple API for storing/retrieving all types that sometimes needs type-hints. See //! tests for implementation examples. pub mod blob_sidecar_list_from_root; -pub mod chunked_iter; -pub mod chunked_vector; pub mod config; pub mod consensus_context; pub mod errors; @@ -21,7 +19,6 @@ mod impls; mod memory_store; pub mod metadata; pub mod metrics; -pub mod partial_beacon_state; pub mod reconstruct; pub mod state_cache; diff --git a/book/src/help_vc.md b/book/src/help_vc.md index b19ff0ba388..2a9936d1d2f 100644 --- a/book/src/help_vc.md +++ b/book/src/help_vc.md @@ -221,6 +221,10 @@ Flags: automatically enabled for <= 64 validators. Enabling this metric for higher validator counts will lead to higher volume of prometheus metrics being collected. + --graffiti-append + When used, client version info will be prepended to user custom + graffiti, with a space in between. This should only be used with a + Lighthouse beacon node. -h, --help Prints help information --http diff --git a/common/eth2/src/lib.rs b/common/eth2/src/lib.rs index 820d817d9d8..8746e3c063c 100644 --- a/common/eth2/src/lib.rs +++ b/common/eth2/src/lib.rs @@ -2207,6 +2207,7 @@ impl BeaconNodeHttpClient { graffiti: Option<&Graffiti>, skip_randao_verification: SkipRandaoVerification, builder_booster_factor: Option, + graffiti_policy: Option, ) -> Result { let mut path = self.eth_path(V3)?; @@ -2234,6 +2235,14 @@ impl BeaconNodeHttpClient { .append_pair("builder_boost_factor", &builder_booster_factor.to_string()); } + // Only append the HTTP URL request if the graffiti_policy is to AppendClientVersions + // If PreserveUserGraffiti (default), then the HTTP URL request does not contain graffiti_policy + // so that the default case is compliant to the spec + if let Some(GraffitiPolicy::AppendClientVersions) = graffiti_policy { + path.query_pairs_mut() + .append_pair("graffiti_policy", "AppendClientVersions"); + } + Ok(path) } @@ -2244,6 +2253,7 @@ impl BeaconNodeHttpClient { randao_reveal: &SignatureBytes, graffiti: Option<&Graffiti>, builder_booster_factor: Option, + graffiti_policy: Option, ) -> Result<(JsonProduceBlockV3Response, ProduceBlockV3Metadata), Error> { self.get_validator_blocks_v3_modular( slot, @@ -2251,6 +2261,7 @@ impl BeaconNodeHttpClient { graffiti, SkipRandaoVerification::No, builder_booster_factor, + graffiti_policy, ) .await } @@ -2263,6 +2274,7 @@ impl BeaconNodeHttpClient { graffiti: Option<&Graffiti>, skip_randao_verification: SkipRandaoVerification, builder_booster_factor: Option, + graffiti_policy: Option, ) -> Result<(JsonProduceBlockV3Response, ProduceBlockV3Metadata), Error> { let path = self .get_validator_blocks_v3_path( @@ -2271,6 +2283,7 @@ impl BeaconNodeHttpClient { graffiti, skip_randao_verification, builder_booster_factor, + graffiti_policy, ) .await?; @@ -2313,6 +2326,7 @@ impl BeaconNodeHttpClient { randao_reveal: &SignatureBytes, graffiti: Option<&Graffiti>, builder_booster_factor: Option, + graffiti_policy: Option, ) -> Result<(ProduceBlockV3Response, ProduceBlockV3Metadata), Error> { self.get_validator_blocks_v3_modular_ssz::( slot, @@ -2320,6 +2334,7 @@ impl BeaconNodeHttpClient { graffiti, SkipRandaoVerification::No, builder_booster_factor, + graffiti_policy, ) .await } @@ -2332,6 +2347,7 @@ impl BeaconNodeHttpClient { graffiti: Option<&Graffiti>, skip_randao_verification: SkipRandaoVerification, builder_booster_factor: Option, + graffiti_policy: Option, ) -> Result<(ProduceBlockV3Response, ProduceBlockV3Metadata), Error> { let path = self .get_validator_blocks_v3_path( @@ -2340,6 +2356,7 @@ impl BeaconNodeHttpClient { graffiti, skip_randao_verification, builder_booster_factor, + graffiti_policy, ) .await?; diff --git a/common/eth2/src/types.rs b/common/eth2/src/types.rs index aace8f936c9..b1a61ce00cc 100644 --- a/common/eth2/src/types.rs +++ b/common/eth2/src/types.rs @@ -752,12 +752,20 @@ pub struct ProposerData { pub slot: Slot, } +#[derive(Clone, Copy, Serialize, Deserialize, Default, Debug)] +pub enum GraffitiPolicy { + #[default] + PreserveUserGraffiti, + AppendClientVersions, +} + #[derive(Clone, Deserialize)] pub struct ValidatorBlocksQuery { pub randao_reveal: SignatureBytes, pub graffiti: Option, pub skip_randao_verification: SkipRandaoVerification, pub builder_boost_factor: Option, + pub graffiti_policy: Option, } #[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Deserialize)] diff --git a/deny.toml b/deny.toml new file mode 100644 index 00000000000..677396c0c34 --- /dev/null +++ b/deny.toml @@ -0,0 +1,23 @@ +# cargo-deny configuration for Lighthouse +# See https://embarkstudios.github.io/cargo-deny/ + +[bans] +# Warn when multiple versions of the same crate are detected +multiple-versions = "warn" +deny = [ + # Legacy Ethereum crates that have been replaced with alloy + { crate = "ethers", reason = "use alloy instead" }, + { crate = "ethereum-types", reason = "use alloy-primitives instead" }, + # Replaced by quick-protobuf + { crate = "protobuf", reason = "use quick-protobuf instead" }, + # Prevent duplicate versions of reqwest - heavy crate with build scripts + { crate = "reqwest", deny-multiple-versions = true, reason = "prevent duplicate versions" }, +] + +[sources] +unknown-registry = "deny" +unknown-git = "warn" +allow-registry = ["https://github.com/rust-lang/crates.io-index"] + +[sources.allow-org] +github = ["sigp"] diff --git a/validator_client/src/cli.rs b/validator_client/src/cli.rs index 477781d3e88..3e1c46097f0 100644 --- a/validator_client/src/cli.rs +++ b/validator_client/src/cli.rs @@ -150,6 +150,16 @@ pub struct ValidatorClient { )] pub graffiti: Option, + #[clap( + long, + requires = "graffiti", + help = "When used, client version info will be prepended to user custom graffiti, with a space in between. \ + This should only be used with a Lighthouse beacon node.", + display_order = 0, + help_heading = FLAG_HEADER + )] + pub graffiti_append: bool, + #[clap( long, value_name = "GRAFFITI-FILE", diff --git a/validator_client/src/config.rs b/validator_client/src/config.rs index 04d69dc9dc1..1a286a74dc1 100644 --- a/validator_client/src/config.rs +++ b/validator_client/src/config.rs @@ -7,7 +7,7 @@ use directory::{ DEFAULT_HARDCODED_NETWORK, DEFAULT_ROOT_DIR, DEFAULT_SECRET_DIR, DEFAULT_VALIDATOR_DIR, get_network_dir, }; -use eth2::types::Graffiti; +use eth2::types::{Graffiti, GraffitiPolicy}; use graffiti_file::GraffitiFile; use initialized_validators::Config as InitializedValidatorsConfig; use lighthouse_validator_store::Config as ValidatorStoreConfig; @@ -55,6 +55,8 @@ pub struct Config { pub graffiti: Option, /// Graffiti file to load per validator graffitis. pub graffiti_file: Option, + /// GraffitiPolicy to append client version info + pub graffiti_policy: Option, /// Configuration for the HTTP REST API. pub http_api: validator_http_api::Config, /// Configuration for the HTTP REST API. @@ -119,6 +121,7 @@ impl Default for Config { long_timeouts_multiplier: 1, graffiti: None, graffiti_file: None, + graffiti_policy: None, http_api: <_>::default(), http_metrics: <_>::default(), beacon_node_fallback: <_>::default(), @@ -233,6 +236,12 @@ impl Config { } } + config.graffiti_policy = if validator_client_config.graffiti_append { + Some(GraffitiPolicy::AppendClientVersions) + } else { + Some(GraffitiPolicy::PreserveUserGraffiti) + }; + if let Some(input_fee_recipient) = validator_client_config.suggested_fee_recipient { config.validator_store.fee_recipient = Some(input_fee_recipient); } diff --git a/validator_client/src/lib.rs b/validator_client/src/lib.rs index 71bdde10b02..23541cf6e28 100644 --- a/validator_client/src/lib.rs +++ b/validator_client/src/lib.rs @@ -486,7 +486,8 @@ impl ProductionValidatorClient { .executor(context.executor.clone()) .chain_spec(context.eth2_config.spec.clone()) .graffiti(config.graffiti) - .graffiti_file(config.graffiti_file.clone()); + .graffiti_file(config.graffiti_file.clone()) + .graffiti_policy(config.graffiti_policy); // If we have proposer nodes, add them to the block service builder. if proposer_nodes_num > 0 { diff --git a/validator_client/validator_services/src/block_service.rs b/validator_client/validator_services/src/block_service.rs index 23658af03fd..625f8db7cb9 100644 --- a/validator_client/validator_services/src/block_service.rs +++ b/validator_client/validator_services/src/block_service.rs @@ -1,5 +1,6 @@ use beacon_node_fallback::{ApiTopic, BeaconNodeFallback, Error as FallbackError, Errors}; use bls::PublicKeyBytes; +use eth2::types::GraffitiPolicy; use eth2::{BeaconNodeHttpClient, StatusCode}; use graffiti_file::{GraffitiFile, determine_graffiti}; use logging::crit; @@ -50,6 +51,7 @@ pub struct BlockServiceBuilder { chain_spec: Option>, graffiti: Option, graffiti_file: Option, + graffiti_policy: Option, } impl BlockServiceBuilder { @@ -63,6 +65,7 @@ impl BlockServiceBuilder { chain_spec: None, graffiti: None, graffiti_file: None, + graffiti_policy: None, } } @@ -106,6 +109,11 @@ impl BlockServiceBuilder { self } + pub fn graffiti_policy(mut self, graffiti_policy: Option) -> Self { + self.graffiti_policy = graffiti_policy; + self + } + pub fn build(self) -> Result, String> { Ok(BlockService { inner: Arc::new(Inner { @@ -127,6 +135,7 @@ impl BlockServiceBuilder { proposer_nodes: self.proposer_nodes, graffiti: self.graffiti, graffiti_file: self.graffiti_file, + graffiti_policy: self.graffiti_policy, }), }) } @@ -192,6 +201,7 @@ pub struct Inner { chain_spec: Arc, graffiti: Option, graffiti_file: Option, + graffiti_policy: Option, } /// Attempts to produce attestations for any block producer(s) at the start of the epoch. @@ -466,6 +476,7 @@ impl BlockService { randao_reveal_ref, graffiti.as_ref(), builder_boost_factor, + self_ref.graffiti_policy, ) .await }) @@ -492,6 +503,7 @@ impl BlockService { randao_reveal_ref, graffiti.as_ref(), builder_boost_factor, + self_ref.graffiti_policy, ) .await .map_err(|e| {