From 283ec8cf2465d547c8b1b06e327f2f915500c927 Mon Sep 17 00:00:00 2001 From: realbigsean Date: Mon, 16 Oct 2023 18:53:46 -0400 Subject: [PATCH] Deneb pr updates 2 (#4851) * use workspace deps in kzg crate * delete unused blobs dp path field * full match on fork name in engine api get payload v3 * only accept v3 payloads on get payload v3 endpoint in mock el * remove FIXMEs related to merge transition tests * move static tx to test utils * default max_per_epoch_activation_churn_limit to mainnet value * remove unnecessary async * remove comment * use task executor in `blob_sidecars` endpoint --- Cargo.lock | 1 - beacon_node/client/src/builder.rs | 3 -- .../execution_layer/src/engine_api/http.rs | 7 ++--- beacon_node/execution_layer/src/lib.rs | 27 +--------------- .../test_utils/execution_block_generator.rs | 31 +++++++++++++++++-- .../src/test_utils/handle_rpc.rs | 15 +-------- .../src/test_utils/mock_execution_layer.rs | 12 ++----- beacon_node/http_api/src/block_id.rs | 6 ++-- beacon_node/http_api/src/lib.rs | 10 +++--- beacon_node/http_api/src/publish_blocks.rs | 3 +- consensus/types/src/chain_spec.rs | 6 +++- crypto/kzg/Cargo.toml | 19 ++++++------ 12 files changed, 60 insertions(+), 80 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 979921fae..70b774064 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3829,7 +3829,6 @@ dependencies = [ "ethereum_ssz_derive", "hex", "serde", - "serde_derive", "tree_hash", ] diff --git a/beacon_node/client/src/builder.rs b/beacon_node/client/src/builder.rs index d0cfb3825..d1184cf75 100644 --- a/beacon_node/client/src/builder.rs +++ b/beacon_node/client/src/builder.rs @@ -71,7 +71,6 @@ pub struct ClientBuilder { gossipsub_registry: Option, db_path: Option, freezer_db_path: Option, - blobs_db_path: Option, http_api_config: http_api::Config, http_metrics_config: http_metrics::Config, slasher: Option>>, @@ -106,7 +105,6 @@ where gossipsub_registry: None, db_path: None, freezer_db_path: None, - blobs_db_path: None, http_api_config: <_>::default(), http_metrics_config: <_>::default(), slasher: None, @@ -927,7 +925,6 @@ where self.db_path = Some(hot_path.into()); self.freezer_db_path = Some(cold_path.into()); - self.blobs_db_path = blobs_path.clone(); let inner_spec = spec.clone(); let deposit_contract_deploy_block = context diff --git a/beacon_node/execution_layer/src/engine_api/http.rs b/beacon_node/execution_layer/src/engine_api/http.rs index c68061710..ac7dfa57e 100644 --- a/beacon_node/execution_layer/src/engine_api/http.rs +++ b/beacon_node/execution_layer/src/engine_api/http.rs @@ -897,10 +897,9 @@ impl HttpJsonRpc { .await?; Ok(JsonGetPayloadResponse::V3(response).into()) } - _ => Err(Error::UnsupportedForkVariant(format!( - "called get_payload_v3 with {}", - fork_name - ))), + ForkName::Base | ForkName::Altair | ForkName::Merge | ForkName::Capella => Err( + Error::UnsupportedForkVariant(format!("called get_payload_v3 with {}", fork_name)), + ), } } diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index 9eb19bb2b..034400487 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -48,7 +48,7 @@ use types::{ AbstractExecPayload, BeaconStateError, BlindedPayload, BlockType, ChainSpec, Epoch, ExecPayload, ExecutionPayloadCapella, ExecutionPayloadDeneb, ExecutionPayloadMerge, }; -use types::{ProposerPreparationData, PublicKeyBytes, Signature, Slot, Transaction}; +use types::{ProposerPreparationData, PublicKeyBytes, Signature, Slot}; mod block_hash; mod engine_api; @@ -2163,31 +2163,6 @@ fn timestamp_now() -> u64 { .as_secs() } -fn static_valid_tx() -> Result, String> { - // This is a real transaction hex encoded, but we don't care about the contents of the transaction. - let transaction: EthersTransaction = serde_json::from_str( - r#"{ - "blockHash":"0x1d59ff54b1eb26b013ce3cb5fc9dab3705b415a67127a003c3e61eb445bb8df2", - "blockNumber":"0x5daf3b", - "from":"0xa7d9ddbe1f17865597fbd27ec712455208b6b76d", - "gas":"0xc350", - "gasPrice":"0x4a817c800", - "hash":"0x88df016429689c079f3b2f6ad39fa052532c56795b733da78a91ebe6a713944b", - "input":"0x68656c6c6f21", - "nonce":"0x15", - "to":"0xf02c1c8e6114b1dbe8937a39260b5b0a374432bb", - "transactionIndex":"0x41", - "value":"0xf3dbb76162000", - "v":"0x25", - "r":"0x1b5e176d927f8e9ab405058b2d2457392da3e20f328b16ddabcebc33eaac5fea", - "s":"0x4ba69724e8f69de52f0125ad8b3c5c2cef33019bac3249e2c0a2192766d1721c" - }"#, - ) - .unwrap(); - VariableList::new(transaction.rlp().to_vec()) - .map_err(|e| format!("Failed to convert transaction to SSZ: {:?}", e)) -} - fn noop( _: &ExecutionLayer, _: PayloadContentsRefTuple, diff --git a/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs b/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs index ebfcd0875..6b638bb7f 100644 --- a/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs +++ b/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs @@ -1,4 +1,5 @@ use crate::engines::ForkchoiceState; +use crate::EthersTransaction; use crate::{ engine_api::{ json_structures::{ @@ -6,13 +7,14 @@ use crate::{ }, ExecutionBlock, PayloadAttributes, PayloadId, PayloadStatusV1, PayloadStatusV1Status, }, - static_valid_tx, ExecutionBlockWithTransactions, + ExecutionBlockWithTransactions, }; use eth2::types::BlobsBundle; use kzg::Kzg; use parking_lot::Mutex; use rand::{rngs::StdRng, Rng, SeedableRng}; use serde::{Deserialize, Serialize}; +use ssz_types::VariableList; use std::collections::HashMap; use std::sync::Arc; use tree_hash::TreeHash; @@ -20,7 +22,7 @@ use tree_hash_derive::TreeHash; use types::{ BlobSidecar, ChainSpec, EthSpec, ExecutionBlockHash, ExecutionPayload, ExecutionPayloadCapella, ExecutionPayloadDeneb, ExecutionPayloadHeader, ExecutionPayloadMerge, ForkName, Hash256, - Transactions, Uint256, + Transaction, Transactions, Uint256, }; use super::DEFAULT_TERMINAL_BLOCK; @@ -681,6 +683,31 @@ pub fn generate_random_blobs( Ok((bundle, transactions.into())) } +fn static_valid_tx() -> Result, String> { + // This is a real transaction hex encoded, but we don't care about the contents of the transaction. + let transaction: EthersTransaction = serde_json::from_str( + r#"{ + "blockHash":"0x1d59ff54b1eb26b013ce3cb5fc9dab3705b415a67127a003c3e61eb445bb8df2", + "blockNumber":"0x5daf3b", + "from":"0xa7d9ddbe1f17865597fbd27ec712455208b6b76d", + "gas":"0xc350", + "gasPrice":"0x4a817c800", + "hash":"0x88df016429689c079f3b2f6ad39fa052532c56795b733da78a91ebe6a713944b", + "input":"0x68656c6c6f21", + "nonce":"0x15", + "to":"0xf02c1c8e6114b1dbe8937a39260b5b0a374432bb", + "transactionIndex":"0x41", + "value":"0xf3dbb76162000", + "v":"0x25", + "r":"0x1b5e176d927f8e9ab405058b2d2457392da3e20f328b16ddabcebc33eaac5fea", + "s":"0x4ba69724e8f69de52f0125ad8b3c5c2cef33019bac3249e2c0a2192766d1721c" + }"#, + ) + .unwrap(); + VariableList::new(transaction.rlp().to_vec()) + .map_err(|e| format!("Failed to convert transaction to SSZ: {:?}", e)) +} + fn payload_id_from_u64(n: u64) -> PayloadId { n.to_le_bytes() } diff --git a/beacon_node/execution_layer/src/test_utils/handle_rpc.rs b/beacon_node/execution_layer/src/test_utils/handle_rpc.rs index e50e6f8d3..9dff1ac00 100644 --- a/beacon_node/execution_layer/src/test_utils/handle_rpc.rs +++ b/beacon_node/execution_layer/src/test_utils/handle_rpc.rs @@ -282,20 +282,6 @@ pub async fn handle_rpc( _ => unreachable!(), }), ENGINE_GET_PAYLOAD_V3 => Ok(match JsonExecutionPayload::from(response) { - JsonExecutionPayload::V1(execution_payload) => { - serde_json::to_value(JsonGetPayloadResponseV1 { - execution_payload, - block_value: DEFAULT_MOCK_EL_PAYLOAD_VALUE_WEI.into(), - }) - .unwrap() - } - JsonExecutionPayload::V2(execution_payload) => { - serde_json::to_value(JsonGetPayloadResponseV2 { - execution_payload, - block_value: DEFAULT_MOCK_EL_PAYLOAD_VALUE_WEI.into(), - }) - .unwrap() - } JsonExecutionPayload::V3(execution_payload) => { serde_json::to_value(JsonGetPayloadResponseV3 { execution_payload, @@ -310,6 +296,7 @@ pub async fn handle_rpc( }) .unwrap() } + _ => unreachable!(), }), _ => unreachable!(), } diff --git a/beacon_node/execution_layer/src/test_utils/mock_execution_layer.rs b/beacon_node/execution_layer/src/test_utils/mock_execution_layer.rs index f1bd89868..2ba51bd67 100644 --- a/beacon_node/execution_layer/src/test_utils/mock_execution_layer.rs +++ b/beacon_node/execution_layer/src/test_utils/mock_execution_layer.rs @@ -103,14 +103,8 @@ impl MockExecutionLayer { justified_hash: None, finalized_hash: None, }; - let payload_attributes = PayloadAttributes::new( - timestamp, - prev_randao, - Address::repeat_byte(42), - // FIXME: think about how to handle different forks here.. - None, - None, - ); + let payload_attributes = + PayloadAttributes::new(timestamp, prev_randao, Address::repeat_byte(42), None, None); // Insert a proposer to ensure the fork choice updated command works. let slot = Slot::new(0); @@ -146,7 +140,6 @@ impl MockExecutionLayer { &payload_attributes, forkchoice_update_params, builder_params, - // FIXME: do we need to consider other forks somehow? ForkName::Merge, &self.spec, ) @@ -181,7 +174,6 @@ impl MockExecutionLayer { &payload_attributes, forkchoice_update_params, builder_params, - // FIXME: do we need to consider other forks somehow? What about withdrawals? ForkName::Merge, &self.spec, ) diff --git a/beacon_node/http_api/src/block_id.rs b/beacon_node/http_api/src/block_id.rs index 545213ca8..45fc651f0 100644 --- a/beacon_node/http_api/src/block_id.rs +++ b/beacon_node/http_api/src/block_id.rs @@ -253,7 +253,7 @@ impl BlockId { } /// Return the `BlobSidecarList` identified by `self`. - pub async fn blob_sidecar_list( + pub fn blob_sidecar_list( &self, chain: &BeaconChain, ) -> Result, warp::Rejection> { @@ -263,12 +263,12 @@ impl BlockId { .map_err(warp_utils::reject::beacon_chain_error) } - pub async fn blob_sidecar_list_filtered( + pub fn blob_sidecar_list_filtered( &self, indices: BlobIndicesQuery, chain: &BeaconChain, ) -> Result, warp::Rejection> { - let blob_sidecar_list = self.blob_sidecar_list(chain).await?; + let blob_sidecar_list = self.blob_sidecar_list(chain)?; let blob_sidecar_list_filtered = match indices.indices { Some(vec) => { let list = blob_sidecar_list diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index 1b83b8531..05c678b25 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -1729,16 +1729,18 @@ pub fn serve( .and(block_id_or_err) .and(warp::query::()) .and(warp::path::end()) + .and(task_spawner_filter.clone()) .and(chain_filter.clone()) .and(warp::header::optional::("accept")) - .and_then( + .then( |block_id: BlockId, indices: api_types::BlobIndicesQuery, + task_spawner: TaskSpawner, chain: Arc>, accept_header: Option| { - async move { + task_spawner.blocking_response_task(Priority::P1, move || { let blob_sidecar_list_filtered = - block_id.blob_sidecar_list_filtered(indices, &chain).await?; + block_id.blob_sidecar_list_filtered(indices, &chain)?; match accept_header { Some(api_types::Accept::Ssz) => Response::builder() .status(200) @@ -1755,7 +1757,7 @@ pub fn serve( )) .into_response()), } - } + }) }, ); diff --git a/beacon_node/http_api/src/publish_blocks.rs b/beacon_node/http_api/src/publish_blocks.rs index cd9ccb6c5..e68691ce8 100644 --- a/beacon_node/http_api/src/publish_blocks.rs +++ b/beacon_node/http_api/src/publish_blocks.rs @@ -75,8 +75,7 @@ pub async fn publish_block block.slot(), "publish_delay" => ?publish_delay); - // Send the block, regardless of whether or not it is valid. The API - // specification is very clear that this is the desired behaviour. + match block.as_ref() { SignedBeaconBlock::Base(_) | SignedBeaconBlock::Altair(_) diff --git a/consensus/types/src/chain_spec.rs b/consensus/types/src/chain_spec.rs index 680db57c2..ed0029785 100644 --- a/consensus/types/src/chain_spec.rs +++ b/consensus/types/src/chain_spec.rs @@ -1019,7 +1019,7 @@ pub struct Config { ejection_balance: u64, #[serde(with = "serde_utils::quoted_u64")] min_per_epoch_churn_limit: u64, - #[serde(default)] + #[serde(default = "default_max_per_epoch_activation_churn_limit")] #[serde(with = "serde_utils::quoted_u64")] max_per_epoch_activation_churn_limit: u64, #[serde(with = "serde_utils::quoted_u64")] @@ -1106,6 +1106,10 @@ fn default_subnets_per_node() -> u8 { 2u8 } +const fn default_max_per_epoch_activation_churn_limit() -> u64 { + 8 +} + const fn default_gossip_max_size() -> u64 { 10485760 } diff --git a/crypto/kzg/Cargo.toml b/crypto/kzg/Cargo.toml index b1e933795..d652ecb4c 100644 --- a/crypto/kzg/Cargo.toml +++ b/crypto/kzg/Cargo.toml @@ -7,18 +7,17 @@ edition = "2021" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] -ethereum_ssz = "0.5.0" -ethereum_ssz_derive = "0.5.3" -tree_hash = "0.5.2" -derivative = "2.1.1" -serde = "1.0.116" -serde_derive = "1.0.116" -ethereum_serde_utils = "0.5.0" -hex = "0.4.2" -ethereum_hashing = "1.0.0-beta.2" +arbitrary = { workspace = true } +ethereum_ssz = { workspace = true } +ethereum_ssz_derive = { workspace = true } +tree_hash = { workspace = true } +derivative = { workspace = true } +serde = { workspace = true } +ethereum_serde_utils = { workspace = true } +hex = { workspace = true } +ethereum_hashing = { workspace = true } c-kzg = { git = "https://github.com/ethereum/c-kzg-4844", rev = "f5f6f863d475847876a2bd5ee252058d37c3a15d" , features = ["mainnet-spec", "serde"]} c_kzg_min = { package = "c-kzg", git = "https://github.com/ethereum//c-kzg-4844", rev = "f5f6f863d475847876a2bd5ee252058d37c3a15d", features = ["minimal-spec", "serde"], optional = true } -arbitrary = { version = "1.0", features = ["derive"] } [features] # TODO(deneb): enabled by default for convenience, would need more cfg magic to disable