From d1661cce23904869a4869f0365a6e789bc628082 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Wed, 17 Apr 2024 08:21:54 +0300 Subject: [PATCH 1/9] update --- beacon_node/beacon_processor/src/lib.rs | 16 ++++++++++++ .../src/peer_manager/mod.rs | 3 +++ .../lighthouse_network/src/rpc/config.rs | 6 +++++ .../lighthouse_network/src/rpc/handler.rs | 3 +++ .../lighthouse_network/src/rpc/methods.rs | 26 +++++++++++++++++++ .../lighthouse_network/src/rpc/protocol.rs | 7 +++++ consensus/types/src/chain_spec.rs | 13 ++++++++++ consensus/types/src/eth_spec.rs | 13 ++++++++++ 8 files changed, 87 insertions(+) diff --git a/beacon_node/beacon_processor/src/lib.rs b/beacon_node/beacon_processor/src/lib.rs index 9b83e9cacbc..4f996a0bb73 100644 --- a/beacon_node/beacon_processor/src/lib.rs +++ b/beacon_node/beacon_processor/src/lib.rs @@ -177,6 +177,10 @@ const MAX_BLOCKS_BY_ROOTS_QUEUE_LEN: usize = 1_024; /// will be stored before we start dropping them. const MAX_BLOBS_BY_ROOTS_QUEUE_LEN: usize = 1_024; +/// The maximum number of queued `DataColumnsByRangeRequest` objects received from the network RPC that +/// will be stored before we start dropping them. +const MAX_DATA_COLUMNS_BY_RANGE_QUEUE_LEN: usize = 1024; + /// Maximum number of `SignedBlsToExecutionChange` messages to queue before dropping them. /// /// This value is set high to accommodate the large spike that is expected immediately after Capella @@ -250,6 +254,7 @@ pub const BLOCKS_BY_RANGE_REQUEST: &str = "blocks_by_range_request"; pub const BLOCKS_BY_ROOTS_REQUEST: &str = "blocks_by_roots_request"; pub const BLOBS_BY_RANGE_REQUEST: &str = "blobs_by_range_request"; pub const BLOBS_BY_ROOTS_REQUEST: &str = "blobs_by_roots_request"; +pub const DATA_COLUMNS_BY_RANGE_REQUEST: &str = "data_columns_by_range_request"; pub const LIGHT_CLIENT_BOOTSTRAP_REQUEST: &str = "light_client_bootstrap"; pub const LIGHT_CLIENT_FINALITY_UPDATE_REQUEST: &str = "light_client_finality_update_request"; pub const LIGHT_CLIENT_OPTIMISTIC_UPDATE_REQUEST: &str = "light_client_optimistic_update_request"; @@ -628,6 +633,7 @@ pub enum Work { BlocksByRootsRequest(AsyncFn), BlobsByRangeRequest(BlockingFn), BlobsByRootsRequest(BlockingFn), + DataColumnsByRangeRequest(BlockingFn), GossipBlsToExecutionChange(BlockingFn), LightClientBootstrapRequest(BlockingFn), LightClientOptimisticUpdateRequest(BlockingFn), @@ -670,6 +676,7 @@ impl Work { Work::BlocksByRootsRequest(_) => BLOCKS_BY_ROOTS_REQUEST, Work::BlobsByRangeRequest(_) => BLOBS_BY_RANGE_REQUEST, Work::BlobsByRootsRequest(_) => BLOBS_BY_ROOTS_REQUEST, + Work::DataColumnsByRangeRequest(_) => DATA_COLUMNS_BY_RANGE_REQUEST, Work::LightClientBootstrapRequest(_) => LIGHT_CLIENT_BOOTSTRAP_REQUEST, Work::LightClientOptimisticUpdateRequest(_) => LIGHT_CLIENT_OPTIMISTIC_UPDATE_REQUEST, Work::LightClientFinalityUpdateRequest(_) => LIGHT_CLIENT_FINALITY_UPDATE_REQUEST, @@ -830,6 +837,7 @@ impl BeaconProcessor { let mut bbroots_queue = FifoQueue::new(MAX_BLOCKS_BY_ROOTS_QUEUE_LEN); let mut blbroots_queue = FifoQueue::new(MAX_BLOBS_BY_ROOTS_QUEUE_LEN); let mut blbrange_queue = FifoQueue::new(MAX_BLOBS_BY_RANGE_QUEUE_LEN); + let mut dcrange_queue = FifoQueue::new(MAX_DATA_COLUMNS_BY_RANGE_QUEUE_LEN); let mut gossip_bls_to_execution_change_queue = FifoQueue::new(MAX_BLS_TO_EXECUTION_CHANGE_QUEUE_LEN); @@ -1131,6 +1139,8 @@ impl BeaconProcessor { self.spawn_worker(item, idle_tx); } else if let Some(item) = blbroots_queue.pop() { self.spawn_worker(item, idle_tx); + } else if let Some(item) = dcrange_queue.pop() { + self.spawn_worker(item, idle_tx); // Check slashings after all other consensus messages so we prioritize // following head. // @@ -1271,6 +1281,9 @@ impl BeaconProcessor { Work::BlobsByRangeRequest { .. } => { blbrange_queue.push(work, work_id, &self.log) } + Work::DataColumnsByRangeRequest { .. } => { + dcrange_queue.push(work, work_id, &self.log) + } Work::LightClientBootstrapRequest { .. } => { lc_bootstrap_queue.push(work, work_id, &self.log) } @@ -1495,6 +1508,9 @@ impl BeaconProcessor { Work::BlocksByRangeRequest(work) | Work::BlocksByRootsRequest(work) => { task_spawner.spawn_async(work) } + Work::DataColumnsByRangeRequest(process_fn) => { + task_spawner.spawn_blocking(process_fn) + } Work::ChainSegmentBackfill(process_fn) => task_spawner.spawn_async(process_fn), Work::ApiRequestP0(process_fn) | Work::ApiRequestP1(process_fn) => match process_fn { BlockingOrAsync::Blocking(process_fn) => task_spawner.spawn_blocking(process_fn), diff --git a/beacon_node/lighthouse_network/src/peer_manager/mod.rs b/beacon_node/lighthouse_network/src/peer_manager/mod.rs index 0d9a7c60dd2..8596fc06db2 100644 --- a/beacon_node/lighthouse_network/src/peer_manager/mod.rs +++ b/beacon_node/lighthouse_network/src/peer_manager/mod.rs @@ -553,6 +553,7 @@ impl PeerManager { Protocol::BlocksByRange => PeerAction::MidToleranceError, Protocol::BlocksByRoot => PeerAction::MidToleranceError, Protocol::BlobsByRange => PeerAction::MidToleranceError, + Protocol::DataColumnsByRange => PeerAction::MidToleranceError, // Lighthouse does not currently make light client requests; therefore, this // is an unexpected scenario. We do not ban the peer for rate limiting. Protocol::LightClientBootstrap => return, @@ -577,6 +578,7 @@ impl PeerManager { Protocol::BlocksByRoot => return, Protocol::BlobsByRange => return, Protocol::BlobsByRoot => return, + Protocol::DataColumnsByRange => return, Protocol::Goodbye => return, Protocol::LightClientBootstrap => return, Protocol::LightClientOptimisticUpdate => return, @@ -597,6 +599,7 @@ impl PeerManager { Protocol::BlocksByRoot => PeerAction::MidToleranceError, Protocol::BlobsByRange => PeerAction::MidToleranceError, Protocol::BlobsByRoot => PeerAction::MidToleranceError, + Protocol::DataColumnsByRange => PeerAction::MidToleranceError, Protocol::LightClientBootstrap => return, Protocol::LightClientOptimisticUpdate => return, Protocol::LightClientFinalityUpdate => return, diff --git a/beacon_node/lighthouse_network/src/rpc/config.rs b/beacon_node/lighthouse_network/src/rpc/config.rs index 08b81c7eae5..c46d5c84138 100644 --- a/beacon_node/lighthouse_network/src/rpc/config.rs +++ b/beacon_node/lighthouse_network/src/rpc/config.rs @@ -91,6 +91,7 @@ pub struct RateLimiterConfig { pub(super) blocks_by_root_quota: Quota, pub(super) blobs_by_range_quota: Quota, pub(super) blobs_by_root_quota: Quota, + pub(super) data_columns_by_range_quota: Quota, pub(super) light_client_bootstrap_quota: Quota, pub(super) light_client_optimistic_update_quota: Quota, pub(super) light_client_finality_update_quota: Quota, @@ -105,6 +106,7 @@ impl RateLimiterConfig { pub const DEFAULT_BLOCKS_BY_ROOT_QUOTA: Quota = Quota::n_every(128, 10); pub const DEFAULT_BLOBS_BY_RANGE_QUOTA: Quota = Quota::n_every(768, 10); pub const DEFAULT_BLOBS_BY_ROOT_QUOTA: Quota = Quota::n_every(128, 10); + pub const DEFAULT_DATA_COLUMNS_BY_RANGE_QUOTA: Quota = Quota::n_every(768, 10); pub const DEFAULT_LIGHT_CLIENT_BOOTSTRAP_QUOTA: Quota = Quota::one_every(10); pub const DEFAULT_LIGHT_CLIENT_OPTIMISTIC_UPDATE_QUOTA: Quota = Quota::one_every(10); pub const DEFAULT_LIGHT_CLIENT_FINALITY_UPDATE_QUOTA: Quota = Quota::one_every(10); @@ -121,6 +123,7 @@ impl Default for RateLimiterConfig { blocks_by_root_quota: Self::DEFAULT_BLOCKS_BY_ROOT_QUOTA, blobs_by_range_quota: Self::DEFAULT_BLOBS_BY_RANGE_QUOTA, blobs_by_root_quota: Self::DEFAULT_BLOBS_BY_ROOT_QUOTA, + data_columns_by_range_quota: Self::DEFAULT_DATA_COLUMNS_BY_RANGE_QUOTA, light_client_bootstrap_quota: Self::DEFAULT_LIGHT_CLIENT_BOOTSTRAP_QUOTA, light_client_optimistic_update_quota: Self::DEFAULT_LIGHT_CLIENT_OPTIMISTIC_UPDATE_QUOTA, @@ -170,6 +173,7 @@ impl FromStr for RateLimiterConfig { let mut blocks_by_root_quota = None; let mut blobs_by_range_quota = None; let mut blobs_by_root_quota = None; + let mut data_columns_by_range_quota = None; let mut light_client_bootstrap_quota = None; let mut light_client_optimistic_update_quota = None; let mut light_client_finality_update_quota = None; @@ -184,6 +188,7 @@ impl FromStr for RateLimiterConfig { Protocol::BlocksByRoot => blocks_by_root_quota = blocks_by_root_quota.or(quota), Protocol::BlobsByRange => blobs_by_range_quota = blobs_by_range_quota.or(quota), Protocol::BlobsByRoot => blobs_by_root_quota = blobs_by_root_quota.or(quota), + Protocol::DataColumnsByRange => data_columns_by_range_quota = data_columns_by_range_quota.or(quota), Protocol::Ping => ping_quota = ping_quota.or(quota), Protocol::MetaData => meta_data_quota = meta_data_quota.or(quota), Protocol::LightClientBootstrap => { @@ -211,6 +216,7 @@ impl FromStr for RateLimiterConfig { blobs_by_range_quota: blobs_by_range_quota .unwrap_or(Self::DEFAULT_BLOBS_BY_RANGE_QUOTA), blobs_by_root_quota: blobs_by_root_quota.unwrap_or(Self::DEFAULT_BLOBS_BY_ROOT_QUOTA), + data_columns_by_range_quota: data_columns_by_range_quota.unwrap_or(Self::DEFAULT_DATA_COLUMNS_BY_RANGE_QUOTA), light_client_bootstrap_quota: light_client_bootstrap_quota .unwrap_or(Self::DEFAULT_LIGHT_CLIENT_BOOTSTRAP_QUOTA), light_client_optimistic_update_quota: light_client_optimistic_update_quota diff --git a/beacon_node/lighthouse_network/src/rpc/handler.rs b/beacon_node/lighthouse_network/src/rpc/handler.rs index df5bbba99c8..1da605b58f8 100644 --- a/beacon_node/lighthouse_network/src/rpc/handler.rs +++ b/beacon_node/lighthouse_network/src/rpc/handler.rs @@ -586,6 +586,9 @@ where if matches!(info.protocol, Protocol::BlobsByRange) { debug!(self.log, "BlobsByRange Response sent"; "duration" => Instant::now().duration_since(info.request_start_time).as_secs()); } + if matches!(info.protocol, Protocol::DataColumnsByRange) { + debug!(self.log, "DataColumnsByRange Response sent"; "duration" => Instant::now().duration_since(info.request_start_time).as_secs()); + } // There is nothing more to process on this substream as it has // been closed. Move on to the next one. diff --git a/beacon_node/lighthouse_network/src/rpc/methods.rs b/beacon_node/lighthouse_network/src/rpc/methods.rs index 1b0486ff771..98f9488a4b1 100644 --- a/beacon_node/lighthouse_network/src/rpc/methods.rs +++ b/beacon_node/lighthouse_network/src/rpc/methods.rs @@ -293,6 +293,22 @@ impl BlobsByRangeRequest { } } +/// Request a number of data columns from a peer. +#[derive(Encode, Decode, Clone, Debug, PartialEq)] +pub struct DataColumnsByRangeRequest { + /// The starting slot to request data columns. + pub start_slot: u64, + + /// The number of slots from the start slot. + pub count: u64 +} + +impl DataColumnsByRangeRequest { + pub fn max_data_columns_requested(&self) -> u64 { + self.count.saturating_mul(E::max_data_columns_per_block() as u64) + } +} + /// Request a number of beacon block roots from a peer. #[superstruct( variants(V1, V2), @@ -388,6 +404,9 @@ pub enum RPCResponse { /// A response to a get BLOBS_BY_RANGE request BlobsByRange(Arc>), + /// A response to a get DATA_COLUMN_BY_RANGE request + DataColumnsByRange(Arc>), + /// A response to a get LIGHT_CLIENT_BOOTSTRAP request. LightClientBootstrap(Arc>), @@ -421,6 +440,9 @@ pub enum ResponseTermination { /// Blobs by root stream termination. BlobsByRoot, + + /// Data columns by range stream termination. + DataColumnsByRange, } /// The structured response containing a result/code indicating success or failure @@ -511,6 +533,7 @@ impl RPCResponse { RPCResponse::BlocksByRoot(_) => Protocol::BlocksByRoot, RPCResponse::BlobsByRange(_) => Protocol::BlobsByRange, RPCResponse::BlobsByRoot(_) => Protocol::BlobsByRoot, + RPCResponse::DataColumnsByRange(_) => Protocol::DataColumnsByRange, RPCResponse::Pong(_) => Protocol::Ping, RPCResponse::MetaData(_) => Protocol::MetaData, RPCResponse::LightClientBootstrap(_) => Protocol::LightClientBootstrap, @@ -553,6 +576,9 @@ impl std::fmt::Display for RPCResponse { RPCResponse::BlobsByRange(blob) => { write!(f, "BlobsByRange: Blob slot: {}", blob.slot()) } + RPCResponse::DataColumnsByRange(data_column) => { + write!(f, "DataColumnsByRange: Data column slot: {}", data_column.slot()) + } RPCResponse::BlobsByRoot(sidecar) => { write!(f, "BlobsByRoot: Blob slot: {}", sidecar.slot()) } diff --git a/beacon_node/lighthouse_network/src/rpc/protocol.rs b/beacon_node/lighthouse_network/src/rpc/protocol.rs index f65586087c2..02a54e2b845 100644 --- a/beacon_node/lighthouse_network/src/rpc/protocol.rs +++ b/beacon_node/lighthouse_network/src/rpc/protocol.rs @@ -243,6 +243,9 @@ pub enum Protocol { /// The `BlobsByRoot` protocol name. #[strum(serialize = "blob_sidecars_by_root")] BlobsByRoot, + /// The `DataColumnsByRange` protocol name. + #[strum(serialize = "data_column_sidecars_by_range")] + DataColumnsByRange, /// The `Ping` protocol name. Ping, /// The `MetaData` protocol name. @@ -268,6 +271,7 @@ impl Protocol { Protocol::BlocksByRoot => Some(ResponseTermination::BlocksByRoot), Protocol::BlobsByRange => Some(ResponseTermination::BlobsByRange), Protocol::BlobsByRoot => Some(ResponseTermination::BlobsByRoot), + Protocol::DataColumnsByRange => Some(ResponseTermination::DataColumnsByRange), Protocol::Ping => None, Protocol::MetaData => None, Protocol::LightClientBootstrap => None, @@ -470,6 +474,7 @@ impl ProtocolId { ::ssz_fixed_len(), ), Protocol::BlobsByRoot => RpcLimits::new(0, spec.max_blobs_by_root_request), + Protocol::DataColumnsByRange => RpcLimits::new(0, spec.max_data_columns_by_range_request), Protocol::Ping => RpcLimits::new( ::ssz_fixed_len(), ::ssz_fixed_len(), @@ -496,6 +501,8 @@ impl ProtocolId { Protocol::BlocksByRoot => rpc_block_limits_by_fork(fork_context.current_fork()), Protocol::BlobsByRange => rpc_blob_limits::(), Protocol::BlobsByRoot => rpc_blob_limits::(), + // TODO(dcrange) check limit + Protocl::DataColumnsByRange => rpc_blob_limits::(), Protocol::Ping => RpcLimits::new( ::ssz_fixed_len(), ::ssz_fixed_len(), diff --git a/consensus/types/src/chain_spec.rs b/consensus/types/src/chain_spec.rs index e9345ab14ea..24675b687b9 100644 --- a/consensus/types/src/chain_spec.rs +++ b/consensus/types/src/chain_spec.rs @@ -217,6 +217,7 @@ pub struct ChainSpec { pub max_blocks_by_root_request: usize, pub max_blocks_by_root_request_deneb: usize, pub max_blobs_by_root_request: usize, + pub max_data_columns_by_range_request: usize, /* * Application params @@ -704,6 +705,11 @@ impl ChainSpec { deneb_fork_version: [0x04, 0x00, 0x00, 0x00], deneb_fork_epoch: Some(Epoch::new(269568)), + /* + * DAS params + */ + max_data_columns_by_range_request: 128, + /* * Electra hard fork params */ @@ -806,6 +812,8 @@ impl ChainSpec { // Deneb deneb_fork_version: [0x04, 0x00, 0x00, 0x01], deneb_fork_epoch: None, + // DAS + max_data_columns_by_range_request: 128, // Electra electra_fork_version: [0x05, 0x00, 0x00, 0x01], electra_fork_epoch: None, @@ -978,6 +986,11 @@ impl ChainSpec { deneb_fork_version: [0x04, 0x00, 0x00, 0x64], deneb_fork_epoch: Some(Epoch::new(889856)), + /* + * DAS params + */ + max_data_columns_by_range_request: 128, + /* * Electra hard fork params */ diff --git a/consensus/types/src/eth_spec.rs b/consensus/types/src/eth_spec.rs index a2972b722a2..41050dc80e3 100644 --- a/consensus/types/src/eth_spec.rs +++ b/consensus/types/src/eth_spec.rs @@ -134,6 +134,11 @@ pub trait EthSpec: /// Must be set to `BytesPerFieldElement * FieldElementsPerBlob`. type BytesPerBlob: Unsigned + Clone + Sync + Send + Debug + PartialEq; + /* + * New for DAS + */ + type MaxDataColumnsPerBlock: Unsigned + Clone + Sync + Send + Debug + PartialEq + Unpin; + /* * New in Electra */ @@ -264,6 +269,11 @@ pub trait EthSpec: Self::MaxBlobsPerBlock::to_usize() } + /// Returns the `MAX_DATA_COLUMNS_PER_BLOCK` constant for this specification. + fn max_data_columns_per_block() -> usize { + Self::MaxDataColumnsPerBlock::to_usize() + } + /// Returns the `MAX_BLOB_COMMITMENTS_PER_BLOCK` constant for this specification. fn max_blob_commitments_per_block() -> usize { Self::MaxBlobCommitmentsPerBlock::to_usize() @@ -327,6 +337,7 @@ impl EthSpec for MainnetEthSpec { type MinGasLimit = U5000; type MaxExtraDataBytes = U32; type MaxBlobsPerBlock = U6; + type MaxDataColumnsPerBlock = U64; type MaxBlobCommitmentsPerBlock = U4096; type BytesPerFieldElement = U32; type FieldElementsPerBlob = U4096; @@ -389,6 +400,7 @@ impl EthSpec for MinimalEthSpec { MaxExtraDataBytes, MaxBlsToExecutionChanges, MaxBlobsPerBlock, + MaxDataColumnsPerBlock, BytesPerFieldElement, ElectraPlaceholder }); @@ -437,6 +449,7 @@ impl EthSpec for GnosisEthSpec { type MaxBlsToExecutionChanges = U16; type MaxWithdrawalsPerPayload = U8; type MaxBlobsPerBlock = U6; + type MaxDataColumnsPerBlock = U64; type MaxBlobCommitmentsPerBlock = U4096; type FieldElementsPerBlob = U4096; type BytesPerFieldElement = U32; From 7032d9a2bef5f7b1b271d53835661badc40a93f1 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Mon, 22 Apr 2024 14:45:26 +0300 Subject: [PATCH 2/9] experiment --- .../src/sync/data_column_request_info.rs | 88 +++++++++++++++++++ beacon_node/network/src/sync/mod.rs | 1 + .../network/src/sync/range_sync/batch.rs | 1 + 3 files changed, 90 insertions(+) create mode 100644 beacon_node/network/src/sync/data_column_request_info.rs diff --git a/beacon_node/network/src/sync/data_column_request_info.rs b/beacon_node/network/src/sync/data_column_request_info.rs new file mode 100644 index 00000000000..acd0a45b14f --- /dev/null +++ b/beacon_node/network/src/sync/data_column_request_info.rs @@ -0,0 +1,88 @@ +use std::{collections::VecDeque, sync::Arc}; +use beacon_chain::block_verification_types::RpcBlock; +use types::{DataColumnSidecar, EthSpec, VariableList}; + +use super::range_sync::ByRangeRequestType; + +#[derive(Debug)] +pub struct DataColumnsRequestInfo { + /// Sidecars we have received awaiting for their corresponding block. + accumulated_sidecars: VecDeque>>, + /// Whether the individual RPC request for data columns is finished or not. + is_sidecars_stream_terminated: bool, +} + +impl DataColumnsRequestInfo { + + pub fn new(request_type: ByRangeRequestType) -> Self { + Self { + accumulated_sidecars: <_>::default(), + is_sidecars_stream_terminated: <_>::default(), + } + } + + pub fn get_request_type(&self) -> ByRangeRequestType { + ByRangeRequestType::DataColumns + } + + pub fn add_sidecar_response(&mut self, sidecar_opt: Option>>) { + match sidecar_opt { + Some(sidecar) => self.accumulated_sidecars.push_back(sidecar), + None => self.is_sidecars_stream_terminated = true, + } + } + + pub fn is_finished(&self) -> bool { + self.is_sidecars_stream_terminated + } + + pub fn into_responses(self, rpc_blocks: Vec>) -> Result>, String> { + let DataColumnsRequestInfo { + accumulated_sidecars, + .. + } = self; + + // TODO slots per epoch + let mut responses = Vec::with_capacity(rpc_blocks.len()); + + for rpc_block in rpc_blocks { + let block = rpc_block.as_block(); + let mut data_column_iter = accumulated_sidecars.into_iter().peekable(); + let mut data_column_list = Vec::with_capacity(E::max_blobs_per_block()); + while { + let pair_next_data_column = data_column_iter + .peek() + .map(|sidecar| sidecar.slot() == block.slot()) + .unwrap_or(false); + pair_next_data_column + } { + data_column_list.push(data_column_iter.next().ok_or("Missing next data data column".to_string())?); + } + + let mut data_columns_buffer = vec![None; E::max_data_columns_per_block()]; + for data_column in data_column_list { + let data_column_index = data_column.index as usize; + let Some(data_column_opt) = data_columns_buffer.get_mut(data_column_index) else { + return Err("Invalid data column index".to_string()); + }; + if data_column_opt.is_some() { + return Err("Repeat data column index".to_string()); + } else { + *data_column_opt = Some(data_column); + } + } + let data_columns = VariableList::from(data_columns_buffer.into_iter().flatten().collect::>()); + responses.push(RpcBlock::new(None, block, None, Some(data_columns)).map_err(|e| format!("{e:?}"))?) + + } + + + + // if accumulated sidecars is not empty, throw an error. + if data_column_iter.next().is_some() { + return Err("Received sidecars that don't pair well".to_string()); + } + + Ok(responses) + } +} \ No newline at end of file diff --git a/beacon_node/network/src/sync/mod.rs b/beacon_node/network/src/sync/mod.rs index 7b244bceceb..4d48c81e28e 100644 --- a/beacon_node/network/src/sync/mod.rs +++ b/beacon_node/network/src/sync/mod.rs @@ -4,6 +4,7 @@ mod backfill_sync; mod block_lookups; mod block_sidecar_coupling; +mod data_column_request_info; pub mod manager; mod network_context; mod peer_sync_info; diff --git a/beacon_node/network/src/sync/range_sync/batch.rs b/beacon_node/network/src/sync/range_sync/batch.rs index 75cb49d176d..f308d1f6415 100644 --- a/beacon_node/network/src/sync/range_sync/batch.rs +++ b/beacon_node/network/src/sync/range_sync/batch.rs @@ -21,6 +21,7 @@ const MAX_BATCH_PROCESSING_ATTEMPTS: u8 = 3; pub enum ByRangeRequestType { BlocksAndBlobs, Blocks, + DataColumns, } /// Allows customisation of the above constants used in other sync methods such as BackFillSync. From 4838a00f64cb80ee8a6a1132117c51286dd6e36d Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Wed, 24 Apr 2024 00:39:25 +0300 Subject: [PATCH 3/9] superstruct changes --- beacon_node/beacon_processor/src/lib.rs | 16 -- .../src/peer_manager/mod.rs | 3 - .../lighthouse_network/src/rpc/config.rs | 6 - .../lighthouse_network/src/rpc/handler.rs | 3 - .../lighthouse_network/src/rpc/methods.rs | 26 -- .../lighthouse_network/src/rpc/protocol.rs | 7 - beacon_node/network/src/sync/mod.rs | 1 - .../network/src/sync/range_sync/batch.rs | 1 - .../src/common/get_attesting_indices.rs | 8 +- .../src/common/get_indexed_attestation.rs | 10 +- .../state_processing/src/consensus_context.rs | 27 +- .../block_signature_verifier.rs | 2 +- .../process_operations.rs | 45 ++-- .../per_block_processing/signature_sets.rs | 4 +- .../verify_attestation.rs | 6 +- consensus/types/src/aggregate_and_proof.rs | 6 +- consensus/types/src/attestation.rs | 254 +++++++++++++++++- consensus/types/src/beacon_block.rs | 5 +- consensus/types/src/eth_spec.rs | 8 + consensus/types/src/indexed_attestation.rs | 2 +- .../types/src/signed_aggregate_and_proof.rs | 2 +- 21 files changed, 324 insertions(+), 118 deletions(-) diff --git a/beacon_node/beacon_processor/src/lib.rs b/beacon_node/beacon_processor/src/lib.rs index af8b5596a15..fee55b39adc 100644 --- a/beacon_node/beacon_processor/src/lib.rs +++ b/beacon_node/beacon_processor/src/lib.rs @@ -177,10 +177,6 @@ const MAX_BLOCKS_BY_ROOTS_QUEUE_LEN: usize = 1_024; /// will be stored before we start dropping them. const MAX_BLOBS_BY_ROOTS_QUEUE_LEN: usize = 1_024; -/// The maximum number of queued `DataColumnsByRangeRequest` objects received from the network RPC that -/// will be stored before we start dropping them. -const MAX_DATA_COLUMNS_BY_RANGE_QUEUE_LEN: usize = 1024; - /// Maximum number of `SignedBlsToExecutionChange` messages to queue before dropping them. /// /// This value is set high to accommodate the large spike that is expected immediately after Capella @@ -254,7 +250,6 @@ pub const BLOCKS_BY_RANGE_REQUEST: &str = "blocks_by_range_request"; pub const BLOCKS_BY_ROOTS_REQUEST: &str = "blocks_by_roots_request"; pub const BLOBS_BY_RANGE_REQUEST: &str = "blobs_by_range_request"; pub const BLOBS_BY_ROOTS_REQUEST: &str = "blobs_by_roots_request"; -pub const DATA_COLUMNS_BY_RANGE_REQUEST: &str = "data_columns_by_range_request"; pub const LIGHT_CLIENT_BOOTSTRAP_REQUEST: &str = "light_client_bootstrap"; pub const LIGHT_CLIENT_FINALITY_UPDATE_REQUEST: &str = "light_client_finality_update_request"; pub const LIGHT_CLIENT_OPTIMISTIC_UPDATE_REQUEST: &str = "light_client_optimistic_update_request"; @@ -633,7 +628,6 @@ pub enum Work { BlocksByRootsRequest(AsyncFn), BlobsByRangeRequest(BlockingFn), BlobsByRootsRequest(BlockingFn), - DataColumnsByRangeRequest(BlockingFn), GossipBlsToExecutionChange(BlockingFn), LightClientBootstrapRequest(BlockingFn), LightClientOptimisticUpdateRequest(BlockingFn), @@ -676,7 +670,6 @@ impl Work { Work::BlocksByRootsRequest(_) => BLOCKS_BY_ROOTS_REQUEST, Work::BlobsByRangeRequest(_) => BLOBS_BY_RANGE_REQUEST, Work::BlobsByRootsRequest(_) => BLOBS_BY_ROOTS_REQUEST, - Work::DataColumnsByRangeRequest(_) => DATA_COLUMNS_BY_RANGE_REQUEST, Work::LightClientBootstrapRequest(_) => LIGHT_CLIENT_BOOTSTRAP_REQUEST, Work::LightClientOptimisticUpdateRequest(_) => LIGHT_CLIENT_OPTIMISTIC_UPDATE_REQUEST, Work::LightClientFinalityUpdateRequest(_) => LIGHT_CLIENT_FINALITY_UPDATE_REQUEST, @@ -837,7 +830,6 @@ impl BeaconProcessor { let mut bbroots_queue = FifoQueue::new(MAX_BLOCKS_BY_ROOTS_QUEUE_LEN); let mut blbroots_queue = FifoQueue::new(MAX_BLOBS_BY_ROOTS_QUEUE_LEN); let mut blbrange_queue = FifoQueue::new(MAX_BLOBS_BY_RANGE_QUEUE_LEN); - let mut dcrange_queue = FifoQueue::new(MAX_DATA_COLUMNS_BY_RANGE_QUEUE_LEN); let mut gossip_bls_to_execution_change_queue = FifoQueue::new(MAX_BLS_TO_EXECUTION_CHANGE_QUEUE_LEN); @@ -1139,8 +1131,6 @@ impl BeaconProcessor { self.spawn_worker(item, idle_tx); } else if let Some(item) = blbroots_queue.pop() { self.spawn_worker(item, idle_tx); - } else if let Some(item) = dcrange_queue.pop() { - self.spawn_worker(item, idle_tx); // Check slashings after all other consensus messages so we prioritize // following head. // @@ -1281,9 +1271,6 @@ impl BeaconProcessor { Work::BlobsByRangeRequest { .. } => { blbrange_queue.push(work, work_id, &self.log) } - Work::DataColumnsByRangeRequest { .. } => { - dcrange_queue.push(work, work_id, &self.log) - } Work::LightClientBootstrapRequest { .. } => { lc_bootstrap_queue.push(work, work_id, &self.log) } @@ -1508,9 +1495,6 @@ impl BeaconProcessor { Work::BlocksByRangeRequest(work) | Work::BlocksByRootsRequest(work) => { task_spawner.spawn_async(work) } - Work::DataColumnsByRangeRequest(process_fn) => { - task_spawner.spawn_blocking(process_fn) - } Work::ChainSegmentBackfill(process_fn) => task_spawner.spawn_async(process_fn), Work::ApiRequestP0(process_fn) | Work::ApiRequestP1(process_fn) => match process_fn { BlockingOrAsync::Blocking(process_fn) => task_spawner.spawn_blocking(process_fn), diff --git a/beacon_node/lighthouse_network/src/peer_manager/mod.rs b/beacon_node/lighthouse_network/src/peer_manager/mod.rs index 8596fc06db2..0d9a7c60dd2 100644 --- a/beacon_node/lighthouse_network/src/peer_manager/mod.rs +++ b/beacon_node/lighthouse_network/src/peer_manager/mod.rs @@ -553,7 +553,6 @@ impl PeerManager { Protocol::BlocksByRange => PeerAction::MidToleranceError, Protocol::BlocksByRoot => PeerAction::MidToleranceError, Protocol::BlobsByRange => PeerAction::MidToleranceError, - Protocol::DataColumnsByRange => PeerAction::MidToleranceError, // Lighthouse does not currently make light client requests; therefore, this // is an unexpected scenario. We do not ban the peer for rate limiting. Protocol::LightClientBootstrap => return, @@ -578,7 +577,6 @@ impl PeerManager { Protocol::BlocksByRoot => return, Protocol::BlobsByRange => return, Protocol::BlobsByRoot => return, - Protocol::DataColumnsByRange => return, Protocol::Goodbye => return, Protocol::LightClientBootstrap => return, Protocol::LightClientOptimisticUpdate => return, @@ -599,7 +597,6 @@ impl PeerManager { Protocol::BlocksByRoot => PeerAction::MidToleranceError, Protocol::BlobsByRange => PeerAction::MidToleranceError, Protocol::BlobsByRoot => PeerAction::MidToleranceError, - Protocol::DataColumnsByRange => PeerAction::MidToleranceError, Protocol::LightClientBootstrap => return, Protocol::LightClientOptimisticUpdate => return, Protocol::LightClientFinalityUpdate => return, diff --git a/beacon_node/lighthouse_network/src/rpc/config.rs b/beacon_node/lighthouse_network/src/rpc/config.rs index c46d5c84138..08b81c7eae5 100644 --- a/beacon_node/lighthouse_network/src/rpc/config.rs +++ b/beacon_node/lighthouse_network/src/rpc/config.rs @@ -91,7 +91,6 @@ pub struct RateLimiterConfig { pub(super) blocks_by_root_quota: Quota, pub(super) blobs_by_range_quota: Quota, pub(super) blobs_by_root_quota: Quota, - pub(super) data_columns_by_range_quota: Quota, pub(super) light_client_bootstrap_quota: Quota, pub(super) light_client_optimistic_update_quota: Quota, pub(super) light_client_finality_update_quota: Quota, @@ -106,7 +105,6 @@ impl RateLimiterConfig { pub const DEFAULT_BLOCKS_BY_ROOT_QUOTA: Quota = Quota::n_every(128, 10); pub const DEFAULT_BLOBS_BY_RANGE_QUOTA: Quota = Quota::n_every(768, 10); pub const DEFAULT_BLOBS_BY_ROOT_QUOTA: Quota = Quota::n_every(128, 10); - pub const DEFAULT_DATA_COLUMNS_BY_RANGE_QUOTA: Quota = Quota::n_every(768, 10); pub const DEFAULT_LIGHT_CLIENT_BOOTSTRAP_QUOTA: Quota = Quota::one_every(10); pub const DEFAULT_LIGHT_CLIENT_OPTIMISTIC_UPDATE_QUOTA: Quota = Quota::one_every(10); pub const DEFAULT_LIGHT_CLIENT_FINALITY_UPDATE_QUOTA: Quota = Quota::one_every(10); @@ -123,7 +121,6 @@ impl Default for RateLimiterConfig { blocks_by_root_quota: Self::DEFAULT_BLOCKS_BY_ROOT_QUOTA, blobs_by_range_quota: Self::DEFAULT_BLOBS_BY_RANGE_QUOTA, blobs_by_root_quota: Self::DEFAULT_BLOBS_BY_ROOT_QUOTA, - data_columns_by_range_quota: Self::DEFAULT_DATA_COLUMNS_BY_RANGE_QUOTA, light_client_bootstrap_quota: Self::DEFAULT_LIGHT_CLIENT_BOOTSTRAP_QUOTA, light_client_optimistic_update_quota: Self::DEFAULT_LIGHT_CLIENT_OPTIMISTIC_UPDATE_QUOTA, @@ -173,7 +170,6 @@ impl FromStr for RateLimiterConfig { let mut blocks_by_root_quota = None; let mut blobs_by_range_quota = None; let mut blobs_by_root_quota = None; - let mut data_columns_by_range_quota = None; let mut light_client_bootstrap_quota = None; let mut light_client_optimistic_update_quota = None; let mut light_client_finality_update_quota = None; @@ -188,7 +184,6 @@ impl FromStr for RateLimiterConfig { Protocol::BlocksByRoot => blocks_by_root_quota = blocks_by_root_quota.or(quota), Protocol::BlobsByRange => blobs_by_range_quota = blobs_by_range_quota.or(quota), Protocol::BlobsByRoot => blobs_by_root_quota = blobs_by_root_quota.or(quota), - Protocol::DataColumnsByRange => data_columns_by_range_quota = data_columns_by_range_quota.or(quota), Protocol::Ping => ping_quota = ping_quota.or(quota), Protocol::MetaData => meta_data_quota = meta_data_quota.or(quota), Protocol::LightClientBootstrap => { @@ -216,7 +211,6 @@ impl FromStr for RateLimiterConfig { blobs_by_range_quota: blobs_by_range_quota .unwrap_or(Self::DEFAULT_BLOBS_BY_RANGE_QUOTA), blobs_by_root_quota: blobs_by_root_quota.unwrap_or(Self::DEFAULT_BLOBS_BY_ROOT_QUOTA), - data_columns_by_range_quota: data_columns_by_range_quota.unwrap_or(Self::DEFAULT_DATA_COLUMNS_BY_RANGE_QUOTA), light_client_bootstrap_quota: light_client_bootstrap_quota .unwrap_or(Self::DEFAULT_LIGHT_CLIENT_BOOTSTRAP_QUOTA), light_client_optimistic_update_quota: light_client_optimistic_update_quota diff --git a/beacon_node/lighthouse_network/src/rpc/handler.rs b/beacon_node/lighthouse_network/src/rpc/handler.rs index 1da605b58f8..df5bbba99c8 100644 --- a/beacon_node/lighthouse_network/src/rpc/handler.rs +++ b/beacon_node/lighthouse_network/src/rpc/handler.rs @@ -586,9 +586,6 @@ where if matches!(info.protocol, Protocol::BlobsByRange) { debug!(self.log, "BlobsByRange Response sent"; "duration" => Instant::now().duration_since(info.request_start_time).as_secs()); } - if matches!(info.protocol, Protocol::DataColumnsByRange) { - debug!(self.log, "DataColumnsByRange Response sent"; "duration" => Instant::now().duration_since(info.request_start_time).as_secs()); - } // There is nothing more to process on this substream as it has // been closed. Move on to the next one. diff --git a/beacon_node/lighthouse_network/src/rpc/methods.rs b/beacon_node/lighthouse_network/src/rpc/methods.rs index 98f9488a4b1..1b0486ff771 100644 --- a/beacon_node/lighthouse_network/src/rpc/methods.rs +++ b/beacon_node/lighthouse_network/src/rpc/methods.rs @@ -293,22 +293,6 @@ impl BlobsByRangeRequest { } } -/// Request a number of data columns from a peer. -#[derive(Encode, Decode, Clone, Debug, PartialEq)] -pub struct DataColumnsByRangeRequest { - /// The starting slot to request data columns. - pub start_slot: u64, - - /// The number of slots from the start slot. - pub count: u64 -} - -impl DataColumnsByRangeRequest { - pub fn max_data_columns_requested(&self) -> u64 { - self.count.saturating_mul(E::max_data_columns_per_block() as u64) - } -} - /// Request a number of beacon block roots from a peer. #[superstruct( variants(V1, V2), @@ -404,9 +388,6 @@ pub enum RPCResponse { /// A response to a get BLOBS_BY_RANGE request BlobsByRange(Arc>), - /// A response to a get DATA_COLUMN_BY_RANGE request - DataColumnsByRange(Arc>), - /// A response to a get LIGHT_CLIENT_BOOTSTRAP request. LightClientBootstrap(Arc>), @@ -440,9 +421,6 @@ pub enum ResponseTermination { /// Blobs by root stream termination. BlobsByRoot, - - /// Data columns by range stream termination. - DataColumnsByRange, } /// The structured response containing a result/code indicating success or failure @@ -533,7 +511,6 @@ impl RPCResponse { RPCResponse::BlocksByRoot(_) => Protocol::BlocksByRoot, RPCResponse::BlobsByRange(_) => Protocol::BlobsByRange, RPCResponse::BlobsByRoot(_) => Protocol::BlobsByRoot, - RPCResponse::DataColumnsByRange(_) => Protocol::DataColumnsByRange, RPCResponse::Pong(_) => Protocol::Ping, RPCResponse::MetaData(_) => Protocol::MetaData, RPCResponse::LightClientBootstrap(_) => Protocol::LightClientBootstrap, @@ -576,9 +553,6 @@ impl std::fmt::Display for RPCResponse { RPCResponse::BlobsByRange(blob) => { write!(f, "BlobsByRange: Blob slot: {}", blob.slot()) } - RPCResponse::DataColumnsByRange(data_column) => { - write!(f, "DataColumnsByRange: Data column slot: {}", data_column.slot()) - } RPCResponse::BlobsByRoot(sidecar) => { write!(f, "BlobsByRoot: Blob slot: {}", sidecar.slot()) } diff --git a/beacon_node/lighthouse_network/src/rpc/protocol.rs b/beacon_node/lighthouse_network/src/rpc/protocol.rs index 02a54e2b845..f65586087c2 100644 --- a/beacon_node/lighthouse_network/src/rpc/protocol.rs +++ b/beacon_node/lighthouse_network/src/rpc/protocol.rs @@ -243,9 +243,6 @@ pub enum Protocol { /// The `BlobsByRoot` protocol name. #[strum(serialize = "blob_sidecars_by_root")] BlobsByRoot, - /// The `DataColumnsByRange` protocol name. - #[strum(serialize = "data_column_sidecars_by_range")] - DataColumnsByRange, /// The `Ping` protocol name. Ping, /// The `MetaData` protocol name. @@ -271,7 +268,6 @@ impl Protocol { Protocol::BlocksByRoot => Some(ResponseTermination::BlocksByRoot), Protocol::BlobsByRange => Some(ResponseTermination::BlobsByRange), Protocol::BlobsByRoot => Some(ResponseTermination::BlobsByRoot), - Protocol::DataColumnsByRange => Some(ResponseTermination::DataColumnsByRange), Protocol::Ping => None, Protocol::MetaData => None, Protocol::LightClientBootstrap => None, @@ -474,7 +470,6 @@ impl ProtocolId { ::ssz_fixed_len(), ), Protocol::BlobsByRoot => RpcLimits::new(0, spec.max_blobs_by_root_request), - Protocol::DataColumnsByRange => RpcLimits::new(0, spec.max_data_columns_by_range_request), Protocol::Ping => RpcLimits::new( ::ssz_fixed_len(), ::ssz_fixed_len(), @@ -501,8 +496,6 @@ impl ProtocolId { Protocol::BlocksByRoot => rpc_block_limits_by_fork(fork_context.current_fork()), Protocol::BlobsByRange => rpc_blob_limits::(), Protocol::BlobsByRoot => rpc_blob_limits::(), - // TODO(dcrange) check limit - Protocl::DataColumnsByRange => rpc_blob_limits::(), Protocol::Ping => RpcLimits::new( ::ssz_fixed_len(), ::ssz_fixed_len(), diff --git a/beacon_node/network/src/sync/mod.rs b/beacon_node/network/src/sync/mod.rs index 4d48c81e28e..7b244bceceb 100644 --- a/beacon_node/network/src/sync/mod.rs +++ b/beacon_node/network/src/sync/mod.rs @@ -4,7 +4,6 @@ mod backfill_sync; mod block_lookups; mod block_sidecar_coupling; -mod data_column_request_info; pub mod manager; mod network_context; mod peer_sync_info; diff --git a/beacon_node/network/src/sync/range_sync/batch.rs b/beacon_node/network/src/sync/range_sync/batch.rs index f308d1f6415..75cb49d176d 100644 --- a/beacon_node/network/src/sync/range_sync/batch.rs +++ b/beacon_node/network/src/sync/range_sync/batch.rs @@ -21,7 +21,6 @@ const MAX_BATCH_PROCESSING_ATTEMPTS: u8 = 3; pub enum ByRangeRequestType { BlocksAndBlobs, Blocks, - DataColumns, } /// Allows customisation of the above constants used in other sync methods such as BackFillSync. diff --git a/consensus/state_processing/src/common/get_attesting_indices.rs b/consensus/state_processing/src/common/get_attesting_indices.rs index a89b71ff2b5..220189c0b10 100644 --- a/consensus/state_processing/src/common/get_attesting_indices.rs +++ b/consensus/state_processing/src/common/get_attesting_indices.rs @@ -27,6 +27,10 @@ pub fn get_attesting_indices_from_state( state: &BeaconState, att: &Attestation, ) -> Result, BeaconStateError> { - let committee = state.get_beacon_committee(att.data.slot, att.data.index)?; - get_attesting_indices::(committee.committee, &att.aggregation_bits) + let committee = state.get_beacon_committee(att.data().slot, att.data().index)?; + match att { + Attestation::Base(att) => get_attesting_indices::(committee.committee, &att.aggregation_bits), + // TODO(eip7549) implement get_attesting_indices for electra + Attestation::Electra(att) => todo!(), + } } diff --git a/consensus/state_processing/src/common/get_indexed_attestation.rs b/consensus/state_processing/src/common/get_indexed_attestation.rs index 9cf689df40f..b214128afaa 100644 --- a/consensus/state_processing/src/common/get_indexed_attestation.rs +++ b/consensus/state_processing/src/common/get_indexed_attestation.rs @@ -11,11 +11,15 @@ pub fn get_indexed_attestation( committee: &[usize], attestation: &Attestation, ) -> Result> { - let attesting_indices = get_attesting_indices::(committee, &attestation.aggregation_bits)?; + let attesting_indices = match attestation { + Attestation::Base(att) => get_attesting_indices::(committee, &att.aggregation_bits)?, + // TODO(eip7549) implement get_attesting_indices for electra + Attestation::Electra(_) => todo!(), + }; Ok(IndexedAttestation { attesting_indices: VariableList::new(attesting_indices)?, - data: attestation.data.clone(), - signature: attestation.signature.clone(), + data: attestation.data().clone(), + signature: attestation.signature().clone(), }) } diff --git a/consensus/state_processing/src/consensus_context.rs b/consensus/state_processing/src/consensus_context.rs index 073d87be85b..772f09859eb 100644 --- a/consensus/state_processing/src/consensus_context.rs +++ b/consensus/state_processing/src/consensus_context.rs @@ -22,7 +22,7 @@ pub struct ConsensusContext { pub current_block_root: Option, /// Cache of indexed attestations constructed during block processing. pub indexed_attestations: - HashMap<(AttestationData, BitList), IndexedAttestation>, + HashMap<(AttestationData, BitList), IndexedAttestation>, } #[derive(Debug, PartialEq, Clone)] @@ -153,16 +153,33 @@ impl ConsensusContext { state: &BeaconState, attestation: &Attestation, ) -> Result<&IndexedAttestation, BlockOperationError> { + + let aggregation_bits = match attestation { + Attestation::Base(attn) => { + let mut extended_aggregation_bits: BitList = BitList::with_capacity(attn.aggregation_bits.len()) + .map_err(BeaconStateError::from)?; + + for (i, bit) in attn.aggregation_bits.iter().enumerate() { + extended_aggregation_bits.set( + i, bit + ).map_err(BeaconStateError::from)?; + } + extended_aggregation_bits + }, + Attestation::Electra(attn) => attn.aggregation_bits.clone(), + }; + + let key = ( - attestation.data.clone(), - attestation.aggregation_bits.clone(), + attestation.data().clone(), + aggregation_bits, ); match self.indexed_attestations.entry(key) { Entry::Occupied(occupied) => Ok(occupied.into_mut()), Entry::Vacant(vacant) => { let committee = - state.get_beacon_committee(attestation.data.slot, attestation.data.index)?; + state.get_beacon_committee(attestation.data().slot, attestation.data().index)?; let indexed_attestation = get_indexed_attestation(committee.committee, attestation)?; Ok(vacant.insert(indexed_attestation)) @@ -178,7 +195,7 @@ impl ConsensusContext { pub fn set_indexed_attestations( mut self, attestations: HashMap< - (AttestationData, BitList), + (AttestationData, BitList), IndexedAttestation, >, ) -> Self { diff --git a/consensus/state_processing/src/per_block_processing/block_signature_verifier.rs b/consensus/state_processing/src/per_block_processing/block_signature_verifier.rs index 3b8a8ea52c9..884fe8305ba 100644 --- a/consensus/state_processing/src/per_block_processing/block_signature_verifier.rs +++ b/consensus/state_processing/src/per_block_processing/block_signature_verifier.rs @@ -290,7 +290,7 @@ where self.sets.push(indexed_attestation_signature_set( self.state, self.get_pubkey.clone(), - &attestation.signature, + attestation.signature(), indexed_attestation, self.spec, )?); diff --git a/consensus/state_processing/src/per_block_processing/process_operations.rs b/consensus/state_processing/src/per_block_processing/process_operations.rs index 63b7c9e01fb..47037c2f9b1 100644 --- a/consensus/state_processing/src/per_block_processing/process_operations.rs +++ b/consensus/state_processing/src/per_block_processing/process_operations.rs @@ -73,24 +73,33 @@ pub mod base { ) .map_err(|e| e.into_with_index(i))?; - let pending_attestation = PendingAttestation { - aggregation_bits: attestation.aggregation_bits.clone(), - data: attestation.data.clone(), - inclusion_delay: state.slot().safe_sub(attestation.data.slot)?.as_u64(), - proposer_index, + match attestation { + Attestation::Base(att) => { + let pending_attestation = PendingAttestation { + aggregation_bits: att.aggregation_bits.clone(), + data: att.data.clone(), + inclusion_delay: state.slot().safe_sub(att.data.slot)?.as_u64(), + proposer_index, + }; + + if attestation.data().target.epoch == state.current_epoch() { + state + .as_base_mut()? + .current_epoch_attestations + .push(pending_attestation)?; + } else { + state + .as_base_mut()? + .previous_epoch_attestations + .push(pending_attestation)?; + } + } + Attestation::Electra(_) => { + // TODO(eip7549) pending attestations are only phase 0 + // so we should just raise a relevant error here + todo!() + } }; - - if attestation.data.target.epoch == state.current_epoch() { - state - .as_base_mut()? - .current_epoch_attestations - .push(pending_attestation)?; - } else { - state - .as_base_mut()? - .previous_epoch_attestations - .push(pending_attestation)?; - } } Ok(()) @@ -139,7 +148,7 @@ pub mod altair_deneb { .attesting_indices; // Matching roots, participation flag indices - let data = &attestation.data; + let data = attestation.data(); let inclusion_delay = state.slot().safe_sub(data.slot)?.as_u64(); let participation_flag_indices = get_attestation_participation_flag_indices(state, data, inclusion_delay, spec)?; diff --git a/consensus/state_processing/src/per_block_processing/signature_sets.rs b/consensus/state_processing/src/per_block_processing/signature_sets.rs index 163b2cff7a9..92b6ed99449 100644 --- a/consensus/state_processing/src/per_block_processing/signature_sets.rs +++ b/consensus/state_processing/src/per_block_processing/signature_sets.rs @@ -425,7 +425,7 @@ where E: EthSpec, F: Fn(usize) -> Option>, { - let slot = signed_aggregate_and_proof.message.aggregate.data.slot; + let slot = signed_aggregate_and_proof.message.aggregate.data().slot; let domain = spec.get_domain( slot.epoch(E::slots_per_epoch()), @@ -458,7 +458,7 @@ where let target_epoch = signed_aggregate_and_proof .message .aggregate - .data + .data() .target .epoch; diff --git a/consensus/state_processing/src/per_block_processing/verify_attestation.rs b/consensus/state_processing/src/per_block_processing/verify_attestation.rs index 73454559dfd..8369f988f73 100644 --- a/consensus/state_processing/src/per_block_processing/verify_attestation.rs +++ b/consensus/state_processing/src/per_block_processing/verify_attestation.rs @@ -22,7 +22,7 @@ pub fn verify_attestation_for_block_inclusion<'ctxt, E: EthSpec>( verify_signatures: VerifySignatures, spec: &ChainSpec, ) -> Result<&'ctxt IndexedAttestation> { - let data = &attestation.data; + let data = attestation.data(); verify!( data.slot.safe_add(spec.min_attestation_inclusion_delay)? <= state.slot(), @@ -66,7 +66,7 @@ pub fn verify_attestation_for_state<'ctxt, E: EthSpec>( verify_signatures: VerifySignatures, spec: &ChainSpec, ) -> Result<&'ctxt IndexedAttestation> { - let data = &attestation.data; + let data = attestation.data(); verify!( data.index < state.get_committee_count_at_slot(data.slot)?, @@ -90,7 +90,7 @@ fn verify_casper_ffg_vote( attestation: &Attestation, state: &BeaconState, ) -> Result<()> { - let data = &attestation.data; + let data = attestation.data(); verify!( data.target.epoch == data.slot.epoch(E::slots_per_epoch()), Invalid::TargetEpochSlotMismatch { diff --git a/consensus/types/src/aggregate_and_proof.rs b/consensus/types/src/aggregate_and_proof.rs index bfbf4d97afd..0297c6d6845 100644 --- a/consensus/types/src/aggregate_and_proof.rs +++ b/consensus/types/src/aggregate_and_proof.rs @@ -53,7 +53,7 @@ impl AggregateAndProof { let selection_proof = selection_proof .unwrap_or_else(|| { SelectionProof::new::( - aggregate.data.slot, + aggregate.data().slot, secret_key, fork, genesis_validators_root, @@ -77,14 +77,14 @@ impl AggregateAndProof { genesis_validators_root: Hash256, spec: &ChainSpec, ) -> bool { - let target_epoch = self.aggregate.data.slot.epoch(E::slots_per_epoch()); + let target_epoch = self.aggregate.data().slot.epoch(E::slots_per_epoch()); let domain = spec.get_domain( target_epoch, Domain::SelectionProof, fork, genesis_validators_root, ); - let message = self.aggregate.data.slot.signing_root(domain); + let message = self.aggregate.data().slot.signing_root(domain); self.selection_proof.verify(validator_pubkey, message) } } diff --git a/consensus/types/src/attestation.rs b/consensus/types/src/attestation.rs index e43077d0591..6a4b5d74aea 100644 --- a/consensus/types/src/attestation.rs +++ b/consensus/types/src/attestation.rs @@ -1,13 +1,17 @@ +use crate::slot_data::SlotData; +use crate::{test_utils::TestRandom, Hash256, Slot}; use derivative::Derivative; +use rand::RngCore; use safe_arith::ArithError; use serde::{Deserialize, Serialize}; +use ssz::Decode; use ssz_derive::{Decode, Encode}; +use ssz_types::BitVector; +use std::hash::{Hash, Hasher}; +use superstruct::superstruct; use test_random_derive::TestRandom; use tree_hash_derive::TreeHash; -use crate::slot_data::SlotData; -use crate::{test_utils::TestRandom, Hash256, Slot}; - use super::{ AggregateSignature, AttestationData, BitList, ChainSpec, Domain, EthSpec, Fork, SecretKey, Signature, SignedRoot, @@ -20,31 +24,253 @@ pub enum Error { SubnetCountIsZero(ArithError), } -/// Details an attestation that can be slashable. -/// -/// Spec v0.12.1 +#[superstruct( + variants(Base, Electra), + variant_attributes( + derive( + Debug, + Clone, + Serialize, + Deserialize, + Decode, + Encode, + TestRandom, + Derivative, + arbitrary::Arbitrary, + TreeHash, + ), + derivative(PartialEq, Hash(bound = "E: EthSpec")), + serde(bound = "E: EthSpec", deny_unknown_fields), + arbitrary(bound = "E: EthSpec"), + ) +)] #[derive( - arbitrary::Arbitrary, Debug, Clone, Serialize, - Deserialize, - Encode, - Decode, TreeHash, - TestRandom, + Encode, Derivative, + Deserialize, + arbitrary::Arbitrary, + PartialEq, )] -#[derivative(PartialEq, Hash(bound = "E: EthSpec"))] -#[serde(bound = "E: EthSpec")] +#[serde(untagged)] +#[tree_hash(enum_behaviour = "transparent")] +#[ssz(enum_behaviour = "transparent")] +#[serde(bound = "E: EthSpec", deny_unknown_fields)] #[arbitrary(bound = "E: EthSpec")] pub struct Attestation { + #[superstruct(only(Base), partial_getter(rename = "aggregation_bits_base"))] pub aggregation_bits: BitList, + #[superstruct(only(Electra), partial_getter(rename = "aggregation_bits_electra"))] + pub aggregation_bits: BitList, pub data: AttestationData, pub signature: AggregateSignature, + #[superstruct(only(Electra))] + pub committee_bits: BitVector, +} + +impl Decode for Attestation { + fn is_ssz_fixed_len() -> bool { + false + } + + fn from_ssz_bytes(bytes: &[u8]) -> Result { + if let Ok(result) = AttestationBase::from_ssz_bytes(bytes) { + return Ok(Attestation::Base(result)); + } + + if let Ok(result) = AttestationElectra::from_ssz_bytes(bytes) { + return Ok(Attestation::Electra(result)); + } + + Err(ssz::DecodeError::BytesInvalid(String::from( + "bytes not valid for any fork variant", + ))) + } +} + +impl TestRandom for Attestation { + fn random_for_test(rng: &mut impl RngCore) -> Self { + let aggregation_bits: BitList = + BitList::random_for_test(rng); + // let committee_bits: BitList = BitList::random_for_test(rng); + let data = AttestationData::random_for_test(rng); + let signature = AggregateSignature::random_for_test(rng); + + Self::Base(AttestationBase { + aggregation_bits, + // committee_bits, + data, + signature, + }) + } +} + +impl Hash for Attestation { + fn hash(&self, state: &mut H) + where + H: Hasher, + { + match self { + Attestation::Base(att) => att.hash(state), + Attestation::Electra(att) => att.hash(state), + } + } } impl Attestation { + /// Aggregate another Attestation into this one. + /// + /// The aggregation bitfields must be disjoint, and the data must be the same. + pub fn aggregate(&mut self, other: &Self) { + match self { + Attestation::Base(att) => { + debug_assert!(other.as_base().is_ok()); + + if let Ok(other) = other.as_base() { + att.aggregate(other) + } + } + Attestation::Electra(att) => { + debug_assert!(other.as_electra().is_ok()); + + if let Ok(other) = other.as_electra() { + att.aggregate(other) + } + } + } + } + + /// Signs `self`, setting the `committee_position`'th bit of `aggregation_bits` to `true`. + /// + /// Returns an `AlreadySigned` error if the `committee_position`'th bit is already `true`. + pub fn sign( + &mut self, + secret_key: &SecretKey, + committee_position: usize, + fork: &Fork, + genesis_validators_root: Hash256, + spec: &ChainSpec, + ) -> Result<(), Error> { + match self { + Attestation::Base(att) => att.sign( + secret_key, + committee_position, + fork, + genesis_validators_root, + spec, + ), + Attestation::Electra(att) => att.sign( + secret_key, + committee_position, + fork, + genesis_validators_root, + spec, + ), + } + } + + /// Returns an `AlreadySigned` error if the `committee_position`'th bit is already `true`. + pub fn add_signature( + &mut self, + signature: &Signature, + committee_position: usize, + ) -> Result<(), Error> { + match self { + Attestation::Base(att) => att.add_signature(signature, committee_position), + Attestation::Electra(att) => att.add_signature(signature, committee_position), + } + } + + pub fn committee_index(&self) -> u64 { + match self { + Attestation::Base(att) => att.data.index, + Attestation::Electra(att) => att.committee_index(), + } + } +} + +impl AttestationElectra { + /// Are the aggregation bitfields of these attestations disjoint? + pub fn signers_disjoint_from(&self, other: &Self) -> bool { + self.aggregation_bits + .intersection(&other.aggregation_bits) + .is_zero() + } + + pub fn committee_index(&self) -> u64 { + *self.get_committee_indices().first().unwrap_or(&0u64) + } + + pub fn get_committee_indices(&self) -> Vec { + self.committee_bits + .iter() + .enumerate() + .filter_map(|(index, bit)| if bit { Some(index as u64) } else { None }) + .collect() + } + + /// Aggregate another Attestation into this one. + /// + /// The aggregation bitfields must be disjoint, and the data must be the same. + pub fn aggregate(&mut self, other: &Self) { + debug_assert_eq!(self.data, other.data); + debug_assert!(self.signers_disjoint_from(other)); + self.aggregation_bits = self.aggregation_bits.union(&other.aggregation_bits); + self.signature.add_assign_aggregate(&other.signature); + } + + /// Signs `self`, setting the `committee_position`'th bit of `aggregation_bits` to `true`. + /// + /// Returns an `AlreadySigned` error if the `committee_position`'th bit is already `true`. + pub fn sign( + &mut self, + secret_key: &SecretKey, + committee_position: usize, + fork: &Fork, + genesis_validators_root: Hash256, + spec: &ChainSpec, + ) -> Result<(), Error> { + let domain = spec.get_domain( + self.data.target.epoch, + Domain::BeaconAttester, + fork, + genesis_validators_root, + ); + let message = self.data.signing_root(domain); + + self.add_signature(&secret_key.sign(message), committee_position) + } + + /// Adds `signature` to `self` and sets the `committee_position`'th bit of `aggregation_bits` to `true`. + /// + /// Returns an `AlreadySigned` error if the `committee_position`'th bit is already `true`. + pub fn add_signature( + &mut self, + signature: &Signature, + committee_position: usize, + ) -> Result<(), Error> { + if self + .aggregation_bits + .get(committee_position) + .map_err(Error::SszTypesError)? + { + Err(Error::AlreadySigned(committee_position)) + } else { + self.aggregation_bits + .set(committee_position, true) + .map_err(Error::SszTypesError)?; + + self.signature.add_assign(signature); + + Ok(()) + } + } +} + +impl AttestationBase { /// Are the aggregation bitfields of these attestations disjoint? pub fn signers_disjoint_from(&self, other: &Self) -> bool { self.aggregation_bits @@ -113,7 +339,7 @@ impl Attestation { impl SlotData for Attestation { fn get_slot(&self) -> Slot { - self.data.slot + self.data().slot } } diff --git a/consensus/types/src/beacon_block.rs b/consensus/types/src/beacon_block.rs index 14874f0204f..848eab2ce6a 100644 --- a/consensus/types/src/beacon_block.rs +++ b/consensus/types/src/beacon_block.rs @@ -9,6 +9,7 @@ use superstruct::superstruct; use test_random_derive::TestRandom; use tree_hash::TreeHash; use tree_hash_derive::TreeHash; +use crate::attestation::AttestationBase; /// A block of the `BeaconChain`. #[superstruct( @@ -350,12 +351,12 @@ impl> BeaconBlockBase { attestation_2: indexed_attestation, }; - let attestation: Attestation = Attestation { + let attestation: Attestation = Attestation::Base(AttestationBase { aggregation_bits: BitList::with_capacity(E::MaxValidatorsPerCommittee::to_usize()) .unwrap(), data: AttestationData::default(), signature: AggregateSignature::empty(), - }; + }); let deposit = Deposit { proof: FixedVector::from_elem(Hash256::zero()), diff --git a/consensus/types/src/eth_spec.rs b/consensus/types/src/eth_spec.rs index feabf007fb3..ad2e7354633 100644 --- a/consensus/types/src/eth_spec.rs +++ b/consensus/types/src/eth_spec.rs @@ -63,6 +63,8 @@ pub trait EthSpec: * Misc */ type MaxValidatorsPerCommittee: Unsigned + Clone + Sync + Send + Debug + PartialEq + Eq; + type MaxValidatorsPerCommitteePerSlot: Unsigned + Clone + Sync + Send + Debug + PartialEq + Eq; + type MaxCommitteesPerSlot: Unsigned + Clone + Sync + Send + Debug + PartialEq + Eq; /* * Time parameters */ @@ -359,6 +361,8 @@ impl EthSpec for MainnetEthSpec { type JustificationBitsLength = U4; type SubnetBitfieldLength = U64; type MaxValidatorsPerCommittee = U2048; + type MaxCommitteesPerSlot = U64; + type MaxValidatorsPerCommitteePerSlot = U131072; type GenesisEpoch = U0; type SlotsPerEpoch = U32; type EpochsPerEth1VotingPeriod = U64; @@ -435,6 +439,8 @@ impl EthSpec for MinimalEthSpec { SubnetBitfieldLength, SyncCommitteeSubnetCount, MaxValidatorsPerCommittee, + MaxCommitteesPerSlot, + MaxValidatorsPerCommitteePerSlot, GenesisEpoch, HistoricalRootsLimit, ValidatorRegistryLimit, @@ -480,6 +486,8 @@ impl EthSpec for GnosisEthSpec { type JustificationBitsLength = U4; type SubnetBitfieldLength = U64; type MaxValidatorsPerCommittee = U2048; + type MaxCommitteesPerSlot = U64; + type MaxValidatorsPerCommitteePerSlot = U131072; type GenesisEpoch = U0; type SlotsPerEpoch = U16; type EpochsPerEth1VotingPeriod = U64; diff --git a/consensus/types/src/indexed_attestation.rs b/consensus/types/src/indexed_attestation.rs index d80b49d55a7..fd3bd99a3e4 100644 --- a/consensus/types/src/indexed_attestation.rs +++ b/consensus/types/src/indexed_attestation.rs @@ -30,7 +30,7 @@ use tree_hash_derive::TreeHash; pub struct IndexedAttestation { /// Lists validator registry indices, not committee indices. #[serde(with = "quoted_variable_list_u64")] - pub attesting_indices: VariableList, + pub attesting_indices: VariableList, pub data: AttestationData, pub signature: AggregateSignature, } diff --git a/consensus/types/src/signed_aggregate_and_proof.rs b/consensus/types/src/signed_aggregate_and_proof.rs index c31c50ea174..491edb3cb0d 100644 --- a/consensus/types/src/signed_aggregate_and_proof.rs +++ b/consensus/types/src/signed_aggregate_and_proof.rs @@ -57,7 +57,7 @@ impl SignedAggregateAndProof { spec, ); - let target_epoch = message.aggregate.data.slot.epoch(E::slots_per_epoch()); + let target_epoch = message.aggregate.data().slot.epoch(E::slots_per_epoch()); let domain = spec.get_domain( target_epoch, Domain::AggregateAndProof, From 3584eb2f665f8acc19785a37daf895934536af79 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Wed, 24 Apr 2024 00:42:47 +0300 Subject: [PATCH 4/9] revert --- .../src/sync/data_column_request_info.rs | 88 ------------------- 1 file changed, 88 deletions(-) delete mode 100644 beacon_node/network/src/sync/data_column_request_info.rs diff --git a/beacon_node/network/src/sync/data_column_request_info.rs b/beacon_node/network/src/sync/data_column_request_info.rs deleted file mode 100644 index acd0a45b14f..00000000000 --- a/beacon_node/network/src/sync/data_column_request_info.rs +++ /dev/null @@ -1,88 +0,0 @@ -use std::{collections::VecDeque, sync::Arc}; -use beacon_chain::block_verification_types::RpcBlock; -use types::{DataColumnSidecar, EthSpec, VariableList}; - -use super::range_sync::ByRangeRequestType; - -#[derive(Debug)] -pub struct DataColumnsRequestInfo { - /// Sidecars we have received awaiting for their corresponding block. - accumulated_sidecars: VecDeque>>, - /// Whether the individual RPC request for data columns is finished or not. - is_sidecars_stream_terminated: bool, -} - -impl DataColumnsRequestInfo { - - pub fn new(request_type: ByRangeRequestType) -> Self { - Self { - accumulated_sidecars: <_>::default(), - is_sidecars_stream_terminated: <_>::default(), - } - } - - pub fn get_request_type(&self) -> ByRangeRequestType { - ByRangeRequestType::DataColumns - } - - pub fn add_sidecar_response(&mut self, sidecar_opt: Option>>) { - match sidecar_opt { - Some(sidecar) => self.accumulated_sidecars.push_back(sidecar), - None => self.is_sidecars_stream_terminated = true, - } - } - - pub fn is_finished(&self) -> bool { - self.is_sidecars_stream_terminated - } - - pub fn into_responses(self, rpc_blocks: Vec>) -> Result>, String> { - let DataColumnsRequestInfo { - accumulated_sidecars, - .. - } = self; - - // TODO slots per epoch - let mut responses = Vec::with_capacity(rpc_blocks.len()); - - for rpc_block in rpc_blocks { - let block = rpc_block.as_block(); - let mut data_column_iter = accumulated_sidecars.into_iter().peekable(); - let mut data_column_list = Vec::with_capacity(E::max_blobs_per_block()); - while { - let pair_next_data_column = data_column_iter - .peek() - .map(|sidecar| sidecar.slot() == block.slot()) - .unwrap_or(false); - pair_next_data_column - } { - data_column_list.push(data_column_iter.next().ok_or("Missing next data data column".to_string())?); - } - - let mut data_columns_buffer = vec![None; E::max_data_columns_per_block()]; - for data_column in data_column_list { - let data_column_index = data_column.index as usize; - let Some(data_column_opt) = data_columns_buffer.get_mut(data_column_index) else { - return Err("Invalid data column index".to_string()); - }; - if data_column_opt.is_some() { - return Err("Repeat data column index".to_string()); - } else { - *data_column_opt = Some(data_column); - } - } - let data_columns = VariableList::from(data_columns_buffer.into_iter().flatten().collect::>()); - responses.push(RpcBlock::new(None, block, None, Some(data_columns)).map_err(|e| format!("{e:?}"))?) - - } - - - - // if accumulated sidecars is not empty, throw an error. - if data_column_iter.next().is_some() { - return Err("Received sidecars that don't pair well".to_string()); - } - - Ok(responses) - } -} \ No newline at end of file From ff386bcd5d7446ecd19fdd8033beef4f8a4e6193 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Wed, 24 Apr 2024 11:33:26 +0300 Subject: [PATCH 5/9] superstruct changes --- .../beacon_chain/src/attestation_simulator.rs | 2 +- .../src/attestation_verification.rs | 70 +++++++++---------- .../beacon_chain/src/beacon_block_reward.rs | 2 +- beacon_node/beacon_chain/src/beacon_chain.rs | 28 ++++---- beacon_node/beacon_chain/src/block_reward.rs | 2 +- .../beacon_chain/src/early_attester_cache.rs | 5 +- .../src/naive_aggregation_pool.rs | 44 +++++++----- .../beacon_chain/src/observed_aggregates.rs | 20 ++++-- beacon_node/beacon_chain/src/test_utils.rs | 23 +++--- .../beacon_chain/src/validator_monitor.rs | 6 +- .../http_api/src/block_packing_efficiency.rs | 42 ++++++----- beacon_node/http_api/src/lib.rs | 12 ++-- .../http_api/src/publish_attestations.rs | 2 +- .../lighthouse_network/src/types/pubsub.rs | 8 ++- .../gossip_methods.rs | 12 ++-- .../src/network_beacon_processor/mod.rs | 2 +- .../src/subnet_service/attestation_subnets.rs | 2 +- .../operation_pool/src/attestation_storage.rs | 31 ++++---- beacon_node/store/src/consensus_context.rs | 9 ++- .../src/common/get_attesting_indices.rs | 6 +- .../src/common/get_indexed_attestation.rs | 2 +- .../state_processing/src/consensus_context.rs | 40 ++++++----- consensus/types/src/attestation.rs | 24 ++++++- consensus/types/src/beacon_block.rs | 2 +- consensus/types/src/eth_spec.rs | 4 +- lcli/src/indexed_attestations.rs | 2 +- slasher/src/attester_record.rs | 2 +- validator_client/src/attestation_service.rs | 20 +++--- validator_client/src/validator_store.rs | 14 ++-- 29 files changed, 258 insertions(+), 180 deletions(-) diff --git a/beacon_node/beacon_chain/src/attestation_simulator.rs b/beacon_node/beacon_chain/src/attestation_simulator.rs index 6453158458a..c97c4490af0 100644 --- a/beacon_node/beacon_chain/src/attestation_simulator.rs +++ b/beacon_node/beacon_chain/src/attestation_simulator.rs @@ -82,7 +82,7 @@ pub fn produce_unaggregated_attestation( // Store the unaggregated attestation in the validator monitor for later processing match chain.produce_unaggregated_attestation(current_slot, beacon_committee_index) { Ok(unaggregated_attestation) => { - let data = &unaggregated_attestation.data; + let data = unaggregated_attestation.data(); debug!( chain.log, diff --git a/beacon_node/beacon_chain/src/attestation_verification.rs b/beacon_node/beacon_chain/src/attestation_verification.rs index f3bde8678e1..873203b2b18 100644 --- a/beacon_node/beacon_chain/src/attestation_verification.rs +++ b/beacon_node/beacon_chain/src/attestation_verification.rs @@ -455,17 +455,17 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> { verify_propagation_slot_range(&chain.slot_clock, attestation, &chain.spec)?; // Check the attestation's epoch matches its target. - if attestation.data.slot.epoch(T::EthSpec::slots_per_epoch()) - != attestation.data.target.epoch + if attestation.data().slot.epoch(T::EthSpec::slots_per_epoch()) + != attestation.data().target.epoch { return Err(Error::InvalidTargetEpoch { - slot: attestation.data.slot, - epoch: attestation.data.target.epoch, + slot: attestation.data().slot, + epoch: attestation.data().target.epoch, }); } // Ensure the valid aggregated attestation has not already been seen locally. - let attestation_data = &attestation.data; + let attestation_data = attestation.data(); let attestation_data_root = attestation_data.tree_hash_root(); if chain @@ -486,7 +486,7 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> { match chain .observed_aggregators .read() - .validator_has_been_observed(attestation.data.target.epoch, aggregator_index as usize) + .validator_has_been_observed(attestation.data().target.epoch, aggregator_index as usize) { Ok(true) => Err(Error::AggregatorAlreadyKnown(aggregator_index)), Ok(false) => Ok(()), @@ -518,7 +518,7 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> { verify_attestation_target_root::(&head_block, attestation)?; // Ensure that the attestation has participants. - if attestation.aggregation_bits.is_zero() { + if attestation.is_aggregation_bits_zero() { Err(Error::EmptyAggregationBitfield) } else { Ok(attestation_data_root) @@ -611,12 +611,12 @@ impl<'a, T: BeaconChainTypes> VerifiedAggregatedAttestation<'a, T> { if chain .observed_aggregators .write() - .observe_validator(attestation.data.target.epoch, aggregator_index as usize) + .observe_validator(attestation.data().target.epoch, aggregator_index as usize) .map_err(BeaconChainError::from)? { return Err(Error::PriorAttestationKnown { validator_index: aggregator_index, - epoch: attestation.data.target.epoch, + epoch: attestation.data().target.epoch, }); } @@ -712,13 +712,13 @@ impl<'a, T: BeaconChainTypes> IndexedUnaggregatedAttestation<'a, T> { attestation: &Attestation, chain: &BeaconChain, ) -> Result<(), Error> { - let attestation_epoch = attestation.data.slot.epoch(T::EthSpec::slots_per_epoch()); + let attestation_epoch = attestation.data().slot.epoch(T::EthSpec::slots_per_epoch()); // Check the attestation's epoch matches its target. - if attestation_epoch != attestation.data.target.epoch { + if attestation_epoch != attestation.data().target.epoch { return Err(Error::InvalidTargetEpoch { - slot: attestation.data.slot, - epoch: attestation.data.target.epoch, + slot: attestation.data().slot, + epoch: attestation.data().target.epoch, }); } @@ -730,7 +730,7 @@ impl<'a, T: BeaconChainTypes> IndexedUnaggregatedAttestation<'a, T> { // Check to ensure that the attestation is "unaggregated". I.e., it has exactly one // aggregation bit set. - let num_aggregation_bits = attestation.aggregation_bits.num_set_bits(); + let num_aggregation_bits = attestation.num_set_aggregation_bits(); if num_aggregation_bits != 1 { return Err(Error::NotExactlyOneAggregationBitSet(num_aggregation_bits)); } @@ -785,12 +785,12 @@ impl<'a, T: BeaconChainTypes> IndexedUnaggregatedAttestation<'a, T> { if chain .observed_gossip_attesters .read() - .validator_has_been_observed(attestation.data.target.epoch, validator_index as usize) + .validator_has_been_observed(attestation.data().target.epoch, validator_index as usize) .map_err(BeaconChainError::from)? { return Err(Error::PriorAttestationKnown { validator_index, - epoch: attestation.data.target.epoch, + epoch: attestation.data().target.epoch, }); } @@ -881,12 +881,12 @@ impl<'a, T: BeaconChainTypes> VerifiedUnaggregatedAttestation<'a, T> { if chain .observed_gossip_attesters .write() - .observe_validator(attestation.data.target.epoch, validator_index as usize) + .observe_validator(attestation.data().target.epoch, validator_index as usize) .map_err(BeaconChainError::from)? { return Err(Error::PriorAttestationKnown { validator_index, - epoch: attestation.data.target.epoch, + epoch: attestation.data().target.epoch, }); } Ok(()) @@ -998,28 +998,28 @@ fn verify_head_block_is_known( let block_opt = chain .canonical_head .fork_choice_read_lock() - .get_block(&attestation.data.beacon_block_root) + .get_block(&attestation.data().beacon_block_root) .or_else(|| { chain .early_attester_cache - .get_proto_block(attestation.data.beacon_block_root) + .get_proto_block(attestation.data().beacon_block_root) }); if let Some(block) = block_opt { // Reject any block that exceeds our limit on skipped slots. if let Some(max_skip_slots) = max_skip_slots { - if attestation.data.slot > block.slot + max_skip_slots { + if attestation.data().slot > block.slot + max_skip_slots { return Err(Error::TooManySkippedSlots { head_block_slot: block.slot, - attestation_slot: attestation.data.slot, + attestation_slot: attestation.data().slot, }); } } Ok(block) - } else if chain.is_pre_finalization_block(attestation.data.beacon_block_root)? { + } else if chain.is_pre_finalization_block(attestation.data().beacon_block_root)? { Err(Error::HeadBlockFinalized { - beacon_block_root: attestation.data.beacon_block_root, + beacon_block_root: attestation.data().beacon_block_root, }) } else { // The block is either: @@ -1029,7 +1029,7 @@ fn verify_head_block_is_known( // 2) A post-finalization block that we don't know about yet. We'll queue // the attestation until the block becomes available (or we time out). Err(Error::UnknownHeadBlock { - beacon_block_root: attestation.data.beacon_block_root, + beacon_block_root: attestation.data().beacon_block_root, }) } } @@ -1043,7 +1043,7 @@ pub fn verify_propagation_slot_range( attestation: &Attestation, spec: &ChainSpec, ) -> Result<(), Error> { - let attestation_slot = attestation.data.slot; + let attestation_slot = attestation.data().slot; let latest_permissible_slot = slot_clock .now_with_future_tolerance(spec.maximum_gossip_clock_disparity()) .ok_or(BeaconChainError::UnableToReadSlot)?; @@ -1127,7 +1127,7 @@ pub fn verify_attestation_target_root( ) -> Result<(), Error> { // Check the attestation target root. let head_block_epoch = head_block.slot.epoch(E::slots_per_epoch()); - let attestation_epoch = attestation.data.slot.epoch(E::slots_per_epoch()); + let attestation_epoch = attestation.data().slot.epoch(E::slots_per_epoch()); if head_block_epoch > attestation_epoch { // The epoch references an invalid head block from a future epoch. // @@ -1140,7 +1140,7 @@ pub fn verify_attestation_target_root( // Reference: // https://github.com/ethereum/eth2.0-specs/pull/2001#issuecomment-699246659 return Err(Error::InvalidTargetRoot { - attestation: attestation.data.target.root, + attestation: attestation.data().target.root, // It is not clear what root we should expect in this case, since the attestation is // fundamentally invalid. expected: None, @@ -1159,9 +1159,9 @@ pub fn verify_attestation_target_root( }; // Reject any attestation with an invalid target root. - if target_root != attestation.data.target.root { + if target_root != attestation.data().target.root { return Err(Error::InvalidTargetRoot { - attestation: attestation.data.target.root, + attestation: attestation.data().target.root, expected: Some(target_root), }); } @@ -1266,8 +1266,8 @@ where T: BeaconChainTypes, F: Fn((BeaconCommittee, CommitteesPerSlot)) -> Result, { - let attestation_epoch = attestation.data.slot.epoch(T::EthSpec::slots_per_epoch()); - let target = &attestation.data.target; + let attestation_epoch = attestation.data().slot.epoch(T::EthSpec::slots_per_epoch()); + let target = &attestation.data().target; // Attestation target must be for a known block. // @@ -1290,12 +1290,12 @@ where let committees_per_slot = committee_cache.committees_per_slot(); Ok(committee_cache - .get_beacon_committee(attestation.data.slot, attestation.data.index) + .get_beacon_committee(attestation.data().slot, attestation.data().index) .map(|committee| map_fn((committee, committees_per_slot))) .unwrap_or_else(|| { Err(Error::NoCommitteeForSlotAndIndex { - slot: attestation.data.slot, - index: attestation.data.index, + slot: attestation.data().slot, + index: attestation.data().index, }) })) }) diff --git a/beacon_node/beacon_chain/src/beacon_block_reward.rs b/beacon_node/beacon_chain/src/beacon_block_reward.rs index 5b70215d225..33567001e3c 100644 --- a/beacon_node/beacon_chain/src/beacon_block_reward.rs +++ b/beacon_node/beacon_chain/src/beacon_block_reward.rs @@ -202,7 +202,7 @@ impl BeaconChain { let mut previous_epoch_participation = state.previous_epoch_participation()?.clone(); for attestation in block.body().attestations() { - let data = &attestation.data; + let data = attestation.data(); let inclusion_delay = state.slot().safe_sub(data.slot)?.as_u64(); // [Modified in Deneb:EIP7045] let participation_flag_indices = get_attestation_participation_flag_indices( diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index c59c5e8ed10..2e40e27e479 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -121,6 +121,7 @@ use store::{ use task_executor::{ShutdownReason, TaskExecutor}; use tokio_stream::Stream; use tree_hash::TreeHash; +use types::attestation::AttestationBase; use types::blob_sidecar::FixedBlobSidecarList; use types::payload::BlockProductionVersion; use types::*; @@ -1670,7 +1671,7 @@ impl BeaconChain { &self, attestation: Attestation, ) -> Result, Error> { - let beacon_block_root = attestation.data.beacon_block_root; + let beacon_block_root = attestation.data().beacon_block_root; match self .canonical_head .fork_choice_read_lock() @@ -1936,7 +1937,8 @@ impl BeaconChain { }; drop(cache_timer); - Ok(Attestation { + // TODO(eip7549) implement electra variant + Ok(Attestation::Base(AttestationBase { aggregation_bits: BitList::with_capacity(committee_len)?, data: AttestationData { slot: request_slot, @@ -1946,7 +1948,7 @@ impl BeaconChain { target, }, signature: AggregateSignature::empty(), - }) + })) } /// Performs the same validation as `Self::verify_unaggregated_attestation_for_gossip`, but for @@ -2158,8 +2160,8 @@ impl BeaconChain { self.log, "Stored unaggregated attestation"; "outcome" => ?outcome, - "index" => attestation.data.index, - "slot" => attestation.data.slot.as_u64(), + "index" => attestation.data().index, + "slot" => attestation.data().slot.as_u64(), ), Err(NaiveAggregationError::SlotTooLow { slot, @@ -2177,8 +2179,8 @@ impl BeaconChain { self.log, "Failed to store unaggregated attestation"; "error" => ?e, - "index" => attestation.data.index, - "slot" => attestation.data.slot.as_u64(), + "index" => attestation.data().index, + "slot" => attestation.data().slot.as_u64(), ); return Err(Error::from(e).into()); } @@ -3746,7 +3748,7 @@ impl BeaconChain { self.log, "Failed to get indexed attestation"; "purpose" => "validator monitor", - "attestation_slot" => attestation.data.slot, + "attestation_slot" => attestation.data().slot, "error" => ?e, ); continue; @@ -3802,7 +3804,7 @@ impl BeaconChain { self.log, "Failed to register observed attestation"; "error" => ?e, - "epoch" => a.data.target.epoch + "epoch" => a.data().target.epoch ); } } @@ -3814,7 +3816,7 @@ impl BeaconChain { self.log, "Failed to get indexed attestation"; "purpose" => "observation", - "attestation_slot" => a.data.slot, + "attestation_slot" => a.data().slot, "error" => ?e, ); continue; @@ -3825,13 +3827,13 @@ impl BeaconChain { for &validator_index in &indexed_attestation.attesting_indices { if let Err(e) = observed_block_attesters - .observe_validator(a.data.target.epoch, validator_index as usize) + .observe_validator(a.data().target.epoch, validator_index as usize) { debug!( self.log, "Failed to register observed block attester"; "error" => ?e, - "epoch" => a.data.target.epoch, + "epoch" => a.data().target.epoch, "validator_index" => validator_index, ) } @@ -3855,7 +3857,7 @@ impl BeaconChain { self.log, "Failed to get indexed attestation"; "purpose" => "slasher", - "attestation_slot" => attestation.data.slot, + "attestation_slot" => attestation.data().slot, "error" => ?e, ); continue; diff --git a/beacon_node/beacon_chain/src/block_reward.rs b/beacon_node/beacon_chain/src/block_reward.rs index fd0cfc7e9bd..44938668fe9 100644 --- a/beacon_node/beacon_chain/src/block_reward.rs +++ b/beacon_node/beacon_chain/src/block_reward.rs @@ -87,7 +87,7 @@ impl BeaconChain { .body() .attestations() .iter() - .map(|a| a.data.clone()) + .map(|a| a.data().clone()) .collect() } else { vec![] diff --git a/beacon_node/beacon_chain/src/early_attester_cache.rs b/beacon_node/beacon_chain/src/early_attester_cache.rs index 79d732f51b1..d7135d94aa2 100644 --- a/beacon_node/beacon_chain/src/early_attester_cache.rs +++ b/beacon_node/beacon_chain/src/early_attester_cache.rs @@ -6,6 +6,7 @@ use crate::{ use parking_lot::RwLock; use proto_array::Block as ProtoBlock; use std::sync::Arc; +use types::attestation::AttestationBase; use types::*; pub struct CacheItem { @@ -122,7 +123,7 @@ impl EarlyAttesterCache { item.committee_lengths .get_committee_length::(request_slot, request_index, spec)?; - let attestation = Attestation { + let attestation = Attestation::Base(AttestationBase { aggregation_bits: BitList::with_capacity(committee_len) .map_err(BeaconStateError::from)?, data: AttestationData { @@ -133,7 +134,7 @@ impl EarlyAttesterCache { target: item.target, }, signature: AggregateSignature::empty(), - }; + }); metrics::inc_counter(&metrics::BEACON_EARLY_ATTESTER_CACHE_HITS); diff --git a/beacon_node/beacon_chain/src/naive_aggregation_pool.rs b/beacon_node/beacon_chain/src/naive_aggregation_pool.rs index 7eeb9bb56f6..0eab13a4c1d 100644 --- a/beacon_node/beacon_chain/src/naive_aggregation_pool.rs +++ b/beacon_node/beacon_chain/src/naive_aggregation_pool.rs @@ -124,13 +124,22 @@ impl AggregateMap for AggregatedAttestationMap { fn insert(&mut self, a: &Self::Value) -> Result { let _timer = metrics::start_timer(&metrics::ATTESTATION_PROCESSING_AGG_POOL_CORE_INSERT); - let set_bits = a - .aggregation_bits - .iter() - .enumerate() - .filter(|(_i, bit)| *bit) - .map(|(i, _bit)| i) - .collect::>(); + let set_bits = match a { + Attestation::Base(att) => att + .aggregation_bits + .iter() + .enumerate() + .filter(|(_i, bit)| *bit) + .map(|(i, _bit)| i) + .collect::>(), + Attestation::Electra(att) => att + .aggregation_bits + .iter() + .enumerate() + .filter(|(_i, bit)| *bit) + .map(|(i, _bit)| i) + .collect::>(), + }; let committee_index = set_bits .first() @@ -141,12 +150,11 @@ impl AggregateMap for AggregatedAttestationMap { return Err(Error::MoreThanOneAggregationBitSet(set_bits.len())); } - let attestation_data_root = a.data.tree_hash_root(); + let attestation_data_root = a.data().tree_hash_root(); if let Some(existing_attestation) = self.map.get_mut(&attestation_data_root) { if existing_attestation - .aggregation_bits - .get(committee_index) + .get_aggregation_bit(committee_index) .map_err(|_| Error::InconsistentBitfieldLengths)? { Ok(InsertOutcome::SignatureAlreadyKnown { committee_index }) @@ -476,8 +484,9 @@ mod tests { fn get_attestation(slot: Slot) -> Attestation { let mut a: Attestation = test_random_instance(); - a.data.slot = slot; - a.aggregation_bits = BitList::with_capacity(4).expect("should create bitlist"); + a.data_mut().slot = slot; + *a.aggregation_bits_base_mut().unwrap() = + BitList::with_capacity(4).expect("should create bitlist"); a } @@ -521,7 +530,8 @@ mod tests { } fn unset_attestation_bit(a: &mut Attestation, i: usize) { - a.aggregation_bits + a.aggregation_bits_base_mut() + .unwrap() .set(i, false) .expect("should unset aggregation bit") } @@ -533,19 +543,19 @@ mod tests { } fn mutate_attestation_block_root(a: &mut Attestation, block_root: Hash256) { - a.data.beacon_block_root = block_root + a.data_mut().beacon_block_root = block_root } fn mutate_attestation_slot(a: &mut Attestation, slot: Slot) { - a.data.slot = slot + a.data_mut().slot = slot } fn attestation_block_root_comparator(a: &Attestation, block_root: Hash256) -> bool { - a.data.beacon_block_root == block_root + a.data().beacon_block_root == block_root } fn key_from_attestation(a: &Attestation) -> AttestationData { - a.data.clone() + a.data().clone() } fn mutate_sync_contribution_block_root( diff --git a/beacon_node/beacon_chain/src/observed_aggregates.rs b/beacon_node/beacon_chain/src/observed_aggregates.rs index ab00aefcd3e..d1e9b12e272 100644 --- a/beacon_node/beacon_chain/src/observed_aggregates.rs +++ b/beacon_node/beacon_chain/src/observed_aggregates.rs @@ -105,21 +105,33 @@ pub trait SubsetItem { impl SubsetItem for Attestation { type Item = BitList; fn is_subset(&self, other: &Self::Item) -> bool { - self.aggregation_bits.is_subset(other) + match self { + Attestation::Base(att) => att.aggregation_bits.is_subset(other), + // TODO(eip7549) implement electra variant + Attestation::Electra(_) => todo!(), + } } fn is_superset(&self, other: &Self::Item) -> bool { - other.is_subset(&self.aggregation_bits) + match self { + Attestation::Base(att) => other.is_subset(&att.aggregation_bits), + // TODO(eip7549) implement electra variant + Attestation::Electra(_) => todo!(), + } } /// Returns the sync contribution aggregation bits. fn get_item(&self) -> Self::Item { - self.aggregation_bits.clone() + match self { + Attestation::Base(att) => att.aggregation_bits.clone(), + // TODO(eip7549) implement electra variant + Attestation::Electra(_) => todo!(), + } } /// Returns the hash tree root of the attestation data. fn root(&self) -> Hash256 { - self.data.tree_hash_root() + self.data().tree_hash_root() } } diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index debc4881a60..0983906cf14 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -61,6 +61,7 @@ use store::{config::StoreConfig, HotColdDB, ItemStore, LevelDB, MemoryStore}; use task_executor::TaskExecutor; use task_executor::{test_utils::TestRuntime, ShutdownReason}; use tree_hash::TreeHash; +use types::attestation::AttestationBase; use types::payload::BlockProductionVersion; pub use types::test_utils::generate_deterministic_keypairs; use types::test_utils::TestRandom; @@ -1037,7 +1038,7 @@ where *state.get_block_root(target_slot)? }; - Ok(Attestation { + Ok(Attestation::Base(AttestationBase { aggregation_bits: BitList::with_capacity(committee_len)?, data: AttestationData { slot, @@ -1050,7 +1051,7 @@ where }, }, signature: AggregateSignature::empty(), - }) + })) } /// A list of attestations for each committee for the given slot. @@ -1125,17 +1126,21 @@ where ) .unwrap(); - attestation.aggregation_bits.set(i, true).unwrap(); + attestation + .aggregation_bits_base_mut() + .unwrap() + .set(i, true) + .unwrap(); - attestation.signature = { + *attestation.signature_mut() = { let domain = self.spec.get_domain( - attestation.data.target.epoch, + attestation.data().target.epoch, Domain::BeaconAttester, &fork, state.genesis_validators_root(), ); - let message = attestation.data.signing_root(domain); + let message = attestation.data().signing_root(domain); let mut agg_sig = AggregateSignature::infinity(); @@ -1147,7 +1152,7 @@ where }; let subnet_id = SubnetId::compute_subnet_for_attestation_data::( - &attestation.data, + attestation.data(), committee_count, &self.chain.spec, ) @@ -1321,7 +1326,7 @@ where // If there are any attestations in this committee, create an aggregate. if let Some((attestation, _)) = committee_attestations.first() { let bc = state - .get_beacon_committee(attestation.data.slot, attestation.data.index) + .get_beacon_committee(attestation.data().slot, attestation.data().index) .unwrap(); // Find an aggregator if one exists. Return `None` if there are no @@ -1352,7 +1357,7 @@ where // aggregate locally. let aggregate = self .chain - .get_aggregated_attestation(&attestation.data) + .get_aggregated_attestation(attestation.data()) .unwrap() .unwrap_or_else(|| { committee_attestations.iter().skip(1).fold( diff --git a/beacon_node/beacon_chain/src/validator_monitor.rs b/beacon_node/beacon_chain/src/validator_monitor.rs index a63940074b4..8da3f680a5c 100644 --- a/beacon_node/beacon_chain/src/validator_monitor.rs +++ b/beacon_node/beacon_chain/src/validator_monitor.rs @@ -469,7 +469,7 @@ impl ValidatorMonitor { unaggregated_attestations.remove(&oldest_slot); } } - let slot = attestation.data.slot; + let slot = attestation.data().slot; self.unaggregated_attestations.insert(slot, attestation); } @@ -730,12 +730,12 @@ impl ValidatorMonitor { // that qualifies the committee index for reward is included let inclusion_delay = spec.min_attestation_inclusion_delay; - let data = &unaggregated_attestation.data; + let data = unaggregated_attestation.data(); // Get the reward indices for the unaggregated attestation or log an error match get_attestation_participation_flag_indices( state, - &unaggregated_attestation.data, + unaggregated_attestation.data(), inclusion_delay, spec, ) { diff --git a/beacon_node/http_api/src/block_packing_efficiency.rs b/beacon_node/http_api/src/block_packing_efficiency.rs index d78f1f7c66e..54df10e45b8 100644 --- a/beacon_node/http_api/src/block_packing_efficiency.rs +++ b/beacon_node/http_api/src/block_packing_efficiency.rs @@ -10,8 +10,8 @@ use std::collections::{HashMap, HashSet}; use std::marker::PhantomData; use std::sync::Arc; use types::{ - BeaconCommittee, BeaconState, BeaconStateError, BlindedPayload, ChainSpec, Epoch, EthSpec, - Hash256, OwnedBeaconCommittee, RelativeEpoch, SignedBeaconBlock, Slot, + Attestation, BeaconCommittee, BeaconState, BeaconStateError, BlindedPayload, ChainSpec, Epoch, + EthSpec, Hash256, OwnedBeaconCommittee, RelativeEpoch, SignedBeaconBlock, Slot, }; use warp_utils::reject::{beacon_chain_error, custom_bad_request, custom_server_error}; @@ -112,21 +112,29 @@ impl PackingEfficiencyHandler { let mut attestations_in_block = HashMap::new(); for attestation in attestations.iter() { - for (position, voted) in attestation.aggregation_bits.iter().enumerate() { - if voted { - let unique_attestation = UniqueAttestation { - slot: attestation.data.slot, - committee_index: attestation.data.index, - committee_position: position, - }; - let inclusion_distance: u64 = block - .slot() - .as_u64() - .checked_sub(attestation.data.slot.as_u64()) - .ok_or(PackingEfficiencyError::InvalidAttestationError)?; - - self.available_attestations.remove(&unique_attestation); - attestations_in_block.insert(unique_attestation, inclusion_distance); + match attestation { + Attestation::Base(attn) => { + for (position, voted) in attn.aggregation_bits.iter().enumerate() { + if voted { + let unique_attestation = UniqueAttestation { + slot: attn.data.slot, + committee_index: attn.data.index, + committee_position: position, + }; + let inclusion_distance: u64 = block + .slot() + .as_u64() + .checked_sub(attn.data.slot.as_u64()) + .ok_or(PackingEfficiencyError::InvalidAttestationError)?; + + self.available_attestations.remove(&unique_attestation); + attestations_in_block.insert(unique_attestation, inclusion_distance); + } + } + } + // TODO(eip7549) implement electra variant + Attestation::Electra(_) => { + todo!() } } } diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index cc117c3fb92..562eb956c1f 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -1803,7 +1803,7 @@ pub fn serve( .naive_aggregation_pool .read() .iter() - .filter(|&att| query_filter(&att.data)) + .filter(|&att| query_filter(att.data())) .cloned(), ); Ok(api_types::GenericResponse::from(attestations)) @@ -3166,7 +3166,7 @@ pub fn serve( chain .produce_unaggregated_attestation(query.slot, query.committee_index) - .map(|attestation| attestation.data) + .map(|attestation| attestation.data().clone()) .map(api_types::GenericResponse::from) .map_err(warp_utils::reject::beacon_chain_error) }) @@ -3368,8 +3368,8 @@ pub fn serve( "error" => format!("{:?}", e), "request_index" => index, "aggregator_index" => aggregate.message.aggregator_index, - "attestation_index" => aggregate.message.aggregate.data.index, - "attestation_slot" => aggregate.message.aggregate.data.slot, + "attestation_index" => aggregate.message.aggregate.data().index, + "attestation_slot" => aggregate.message.aggregate.data().slot, ); failures.push(api_types::Failure::new(index, format!("Verification: {:?}", e))); } @@ -3389,8 +3389,8 @@ pub fn serve( "error" => format!("{:?}", e), "request_index" => index, "aggregator_index" => verified_aggregate.aggregate().message.aggregator_index, - "attestation_index" => verified_aggregate.attestation().data.index, - "attestation_slot" => verified_aggregate.attestation().data.slot, + "attestation_index" => verified_aggregate.attestation().data().index, + "attestation_slot" => verified_aggregate.attestation().data().slot, ); failures.push(api_types::Failure::new(index, format!("Fork choice: {:?}", e))); } diff --git a/beacon_node/http_api/src/publish_attestations.rs b/beacon_node/http_api/src/publish_attestations.rs index ed7f1ed17c9..8eaee093c1a 100644 --- a/beacon_node/http_api/src/publish_attestations.rs +++ b/beacon_node/http_api/src/publish_attestations.rs @@ -141,7 +141,7 @@ pub async fn publish_attestations( // move the `attestations` vec into the blocking task, so this small overhead is unavoidable. let attestation_metadata = attestations .iter() - .map(|att| (att.data.slot, att.data.index)) + .map(|att| (att.data().slot, att.data().index)) .collect::>(); // Gossip validate and publish attestations that can be immediately processed. diff --git a/beacon_node/lighthouse_network/src/types/pubsub.rs b/beacon_node/lighthouse_network/src/types/pubsub.rs index a5cf03412d5..653e264349e 100644 --- a/beacon_node/lighthouse_network/src/types/pubsub.rs +++ b/beacon_node/lighthouse_network/src/types/pubsub.rs @@ -343,14 +343,16 @@ impl std::fmt::Display for PubsubMessage { PubsubMessage::AggregateAndProofAttestation(att) => write!( f, "Aggregate and Proof: slot: {}, index: {}, aggregator_index: {}", - att.message.aggregate.data.slot, - att.message.aggregate.data.index, + att.message.aggregate.data().slot, + att.message.aggregate.data().index, att.message.aggregator_index, ), PubsubMessage::Attestation(data) => write!( f, "Attestation: subnet_id: {}, attestation_slot: {}, attestation_index: {}", - *data.0, data.1.data.slot, data.1.data.index, + *data.0, + data.1.data().slot, + data.1.data().index, ), PubsubMessage::VoluntaryExit(_data) => write!(f, "Voluntary Exit"), PubsubMessage::ProposerSlashing(_data) => write!(f, "Proposer Slashing"), diff --git a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs index 7b8826bd853..3897d0a0841 100644 --- a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs @@ -133,7 +133,7 @@ enum FailedAtt { impl FailedAtt { pub fn beacon_block_root(&self) -> &Hash256 { - &self.attestation().data.beacon_block_root + &self.attestation().data().beacon_block_root } pub fn kind(&self) -> &'static str { @@ -412,7 +412,7 @@ impl NetworkBeaconProcessor { reprocess_tx: Option>, seen_timestamp: Duration, ) { - let beacon_block_root = aggregate.message.aggregate.data.beacon_block_root; + let beacon_block_root = aggregate.message.aggregate.data().beacon_block_root; let result = match self .chain @@ -2282,7 +2282,7 @@ impl NetworkBeaconProcessor { self.log, "Ignored attestation to finalized block"; "block_root" => ?beacon_block_root, - "attestation_slot" => failed_att.attestation().data.slot, + "attestation_slot" => failed_att.attestation().data().slot, ); self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Ignore); @@ -2305,9 +2305,9 @@ impl NetworkBeaconProcessor { debug!( self.log, "Dropping attestation"; - "target_root" => ?failed_att.attestation().data.target.root, + "target_root" => ?failed_att.attestation().data().target.root, "beacon_block_root" => ?beacon_block_root, - "slot" => ?failed_att.attestation().data.slot, + "slot" => ?failed_att.attestation().data().slot, "type" => ?attestation_type, "error" => ?e, "peer_id" => % peer_id @@ -2326,7 +2326,7 @@ impl NetworkBeaconProcessor { self.log, "Unable to validate attestation"; "beacon_block_root" => ?beacon_block_root, - "slot" => ?failed_att.attestation().data.slot, + "slot" => ?failed_att.attestation().data().slot, "type" => ?attestation_type, "peer_id" => %peer_id, "error" => ?e, diff --git a/beacon_node/network/src/network_beacon_processor/mod.rs b/beacon_node/network/src/network_beacon_processor/mod.rs index f10646c7414..b85be1b0c15 100644 --- a/beacon_node/network/src/network_beacon_processor/mod.rs +++ b/beacon_node/network/src/network_beacon_processor/mod.rs @@ -144,7 +144,7 @@ impl NetworkBeaconProcessor { processor.process_gossip_aggregate_batch(aggregates, Some(reprocess_tx)) }; - let beacon_block_root = aggregate.message.aggregate.data.beacon_block_root; + let beacon_block_root = aggregate.message.aggregate.data().beacon_block_root; self.try_send(BeaconWorkEvent { drop_during_sync: true, work: Work::GossipAggregate { diff --git a/beacon_node/network/src/subnet_service/attestation_subnets.rs b/beacon_node/network/src/subnet_service/attestation_subnets.rs index ab9ffb95a6c..afe9815d674 100644 --- a/beacon_node/network/src/subnet_service/attestation_subnets.rs +++ b/beacon_node/network/src/subnet_service/attestation_subnets.rs @@ -389,7 +389,7 @@ impl AttestationService { .map(|tracked_vals| { tracked_vals.contains_key(&ExactSubnet { subnet_id: subnet, - slot: attestation.data.slot, + slot: attestation.data().slot, }) }) .unwrap_or(true) diff --git a/beacon_node/operation_pool/src/attestation_storage.rs b/beacon_node/operation_pool/src/attestation_storage.rs index 43fdf3923bd..5641324aa91 100644 --- a/beacon_node/operation_pool/src/attestation_storage.rs +++ b/beacon_node/operation_pool/src/attestation_storage.rs @@ -2,8 +2,8 @@ use crate::AttestationStats; use itertools::Itertools; use std::collections::HashMap; use types::{ - AggregateSignature, Attestation, AttestationData, BeaconState, BitList, Checkpoint, Epoch, - EthSpec, Hash256, Slot, + attestation::AttestationBase, AggregateSignature, Attestation, AttestationData, BeaconState, + BitList, Checkpoint, Epoch, EthSpec, Hash256, Slot, }; #[derive(Debug, PartialEq, Eq, Hash, Clone, Copy)] @@ -54,19 +54,26 @@ pub struct AttestationDataMap { impl SplitAttestation { pub fn new(attestation: Attestation, attesting_indices: Vec) -> Self { let checkpoint = CheckpointKey { - source: attestation.data.source, - target_epoch: attestation.data.target.epoch, + source: attestation.data().source, + target_epoch: attestation.data().target.epoch, }; let data = CompactAttestationData { - slot: attestation.data.slot, - index: attestation.data.index, - beacon_block_root: attestation.data.beacon_block_root, - target_root: attestation.data.target.root, + slot: attestation.data().slot, + index: attestation.data().index, + beacon_block_root: attestation.data().beacon_block_root, + target_root: attestation.data().target.root, }; + + let aggregation_bits = match attestation.clone() { + Attestation::Base(attn) => attn.aggregation_bits, + // TODO(eip7549) implement electra variant + Attestation::Electra(_) => todo!(), + }; + let indexed = CompactIndexedAttestation { attesting_indices, - aggregation_bits: attestation.aggregation_bits, - signature: attestation.signature, + aggregation_bits, + signature: attestation.signature().clone(), }; Self { checkpoint, @@ -99,11 +106,11 @@ impl<'a, E: EthSpec> AttestationRef<'a, E> { } pub fn clone_as_attestation(&self) -> Attestation { - Attestation { + Attestation::Base(AttestationBase { aggregation_bits: self.indexed.aggregation_bits.clone(), data: self.attestation_data(), signature: self.indexed.signature.clone(), - } + }) } } diff --git a/beacon_node/store/src/consensus_context.rs b/beacon_node/store/src/consensus_context.rs index 08fad17b14b..727423d172f 100644 --- a/beacon_node/store/src/consensus_context.rs +++ b/beacon_node/store/src/consensus_context.rs @@ -21,8 +21,13 @@ pub struct OnDiskConsensusContext { /// /// They are not part of the on-disk format. #[ssz(skip_serializing, skip_deserializing)] - indexed_attestations: - HashMap<(AttestationData, BitList), IndexedAttestation>, + indexed_attestations: HashMap< + ( + AttestationData, + BitList, + ), + IndexedAttestation, + >, } impl OnDiskConsensusContext { diff --git a/consensus/state_processing/src/common/get_attesting_indices.rs b/consensus/state_processing/src/common/get_attesting_indices.rs index 220189c0b10..7a1f9f3f480 100644 --- a/consensus/state_processing/src/common/get_attesting_indices.rs +++ b/consensus/state_processing/src/common/get_attesting_indices.rs @@ -29,8 +29,10 @@ pub fn get_attesting_indices_from_state( ) -> Result, BeaconStateError> { let committee = state.get_beacon_committee(att.data().slot, att.data().index)?; match att { - Attestation::Base(att) => get_attesting_indices::(committee.committee, &att.aggregation_bits), + Attestation::Base(att) => { + get_attesting_indices::(committee.committee, &att.aggregation_bits) + } // TODO(eip7549) implement get_attesting_indices for electra - Attestation::Electra(att) => todo!(), + Attestation::Electra(_) => todo!(), } } diff --git a/consensus/state_processing/src/common/get_indexed_attestation.rs b/consensus/state_processing/src/common/get_indexed_attestation.rs index b214128afaa..7643a38c928 100644 --- a/consensus/state_processing/src/common/get_indexed_attestation.rs +++ b/consensus/state_processing/src/common/get_indexed_attestation.rs @@ -12,7 +12,7 @@ pub fn get_indexed_attestation( attestation: &Attestation, ) -> Result> { let attesting_indices = match attestation { - Attestation::Base(att) => get_attesting_indices::(committee, &att.aggregation_bits)?, + Attestation::Base(att) => get_attesting_indices::(committee, &att.aggregation_bits)?, // TODO(eip7549) implement get_attesting_indices for electra Attestation::Electra(_) => todo!(), }; diff --git a/consensus/state_processing/src/consensus_context.rs b/consensus/state_processing/src/consensus_context.rs index 772f09859eb..b1196c942d3 100644 --- a/consensus/state_processing/src/consensus_context.rs +++ b/consensus/state_processing/src/consensus_context.rs @@ -21,8 +21,13 @@ pub struct ConsensusContext { /// Block root of the block at `slot`. pub current_block_root: Option, /// Cache of indexed attestations constructed during block processing. - pub indexed_attestations: - HashMap<(AttestationData, BitList), IndexedAttestation>, + pub indexed_attestations: HashMap< + ( + AttestationData, + BitList, + ), + IndexedAttestation, + >, } #[derive(Debug, PartialEq, Clone)] @@ -153,33 +158,29 @@ impl ConsensusContext { state: &BeaconState, attestation: &Attestation, ) -> Result<&IndexedAttestation, BlockOperationError> { - let aggregation_bits = match attestation { Attestation::Base(attn) => { - let mut extended_aggregation_bits: BitList = BitList::with_capacity(attn.aggregation_bits.len()) - .map_err(BeaconStateError::from)?; - + let mut extended_aggregation_bits: BitList = + BitList::with_capacity(attn.aggregation_bits.len()) + .map_err(BeaconStateError::from)?; + for (i, bit) in attn.aggregation_bits.iter().enumerate() { - extended_aggregation_bits.set( - i, bit - ).map_err(BeaconStateError::from)?; + extended_aggregation_bits + .set(i, bit) + .map_err(BeaconStateError::from)?; } extended_aggregation_bits - }, + } Attestation::Electra(attn) => attn.aggregation_bits.clone(), }; - - let key = ( - attestation.data().clone(), - aggregation_bits, - ); + let key = (attestation.data().clone(), aggregation_bits); match self.indexed_attestations.entry(key) { Entry::Occupied(occupied) => Ok(occupied.into_mut()), Entry::Vacant(vacant) => { - let committee = - state.get_beacon_committee(attestation.data().slot, attestation.data().index)?; + let committee = state + .get_beacon_committee(attestation.data().slot, attestation.data().index)?; let indexed_attestation = get_indexed_attestation(committee.committee, attestation)?; Ok(vacant.insert(indexed_attestation)) @@ -195,7 +196,10 @@ impl ConsensusContext { pub fn set_indexed_attestations( mut self, attestations: HashMap< - (AttestationData, BitList), + ( + AttestationData, + BitList, + ), IndexedAttestation, >, ) -> Self { diff --git a/consensus/types/src/attestation.rs b/consensus/types/src/attestation.rs index 6a4b5d74aea..e036f958fff 100644 --- a/consensus/types/src/attestation.rs +++ b/consensus/types/src/attestation.rs @@ -93,8 +93,7 @@ impl Decode for Attestation { impl TestRandom for Attestation { fn random_for_test(rng: &mut impl RngCore) -> Self { - let aggregation_bits: BitList = - BitList::random_for_test(rng); + let aggregation_bits: BitList = BitList::random_for_test(rng); // let committee_bits: BitList = BitList::random_for_test(rng); let data = AttestationData::random_for_test(rng); let signature = AggregateSignature::random_for_test(rng); @@ -190,6 +189,27 @@ impl Attestation { Attestation::Electra(att) => att.committee_index(), } } + + pub fn is_aggregation_bits_zero(&self) -> bool { + match self { + Attestation::Base(att) => att.aggregation_bits.is_zero(), + Attestation::Electra(att) => att.aggregation_bits.is_zero(), + } + } + + pub fn num_set_aggregation_bits(&self) -> usize { + match self { + Attestation::Base(att) => att.aggregation_bits.num_set_bits(), + Attestation::Electra(att) => att.aggregation_bits.num_set_bits(), + } + } + + pub fn get_aggregation_bit(&self, index: usize) -> Result { + match self { + Attestation::Base(att) => att.aggregation_bits.get(index), + Attestation::Electra(att) => att.aggregation_bits.get(index), + } + } } impl AttestationElectra { diff --git a/consensus/types/src/beacon_block.rs b/consensus/types/src/beacon_block.rs index 848eab2ce6a..cf8c45e07f4 100644 --- a/consensus/types/src/beacon_block.rs +++ b/consensus/types/src/beacon_block.rs @@ -1,3 +1,4 @@ +use crate::attestation::AttestationBase; use crate::test_utils::TestRandom; use crate::*; use derivative::Derivative; @@ -9,7 +10,6 @@ use superstruct::superstruct; use test_random_derive::TestRandom; use tree_hash::TreeHash; use tree_hash_derive::TreeHash; -use crate::attestation::AttestationBase; /// A block of the `BeaconChain`. #[superstruct( diff --git a/consensus/types/src/eth_spec.rs b/consensus/types/src/eth_spec.rs index ad2e7354633..2f27209b9fb 100644 --- a/consensus/types/src/eth_spec.rs +++ b/consensus/types/src/eth_spec.rs @@ -138,8 +138,8 @@ pub trait EthSpec: type BytesPerBlob: Unsigned + Clone + Sync + Send + Debug + PartialEq; /* - * New for DAS - */ + * New for DAS + */ type MaxDataColumnsPerBlock: Unsigned + Clone + Sync + Send + Debug + PartialEq + Unpin; /* diff --git a/lcli/src/indexed_attestations.rs b/lcli/src/indexed_attestations.rs index 63f8cd94637..4c8b06a5088 100644 --- a/lcli/src/indexed_attestations.rs +++ b/lcli/src/indexed_attestations.rs @@ -34,7 +34,7 @@ pub fn run(matches: &ArgMatches) -> Result<(), String> { let indexed_attestations = attestations .into_iter() .map(|att| { - let committee = state.get_beacon_committee(att.data.slot, att.data.index)?; + let committee = state.get_beacon_committee(att.data().slot, att.data().index)?; get_indexed_attestation(committee.committee, &att) }) .collect::, _>>() diff --git a/slasher/src/attester_record.rs b/slasher/src/attester_record.rs index 56fdcb809ff..050849e33f1 100644 --- a/slasher/src/attester_record.rs +++ b/slasher/src/attester_record.rs @@ -80,7 +80,7 @@ impl IndexedAttesterRecord { #[derive(Debug, Clone, Encode, Decode, TreeHash)] struct IndexedAttestationHeader { - pub attesting_indices: VariableList, + pub attesting_indices: VariableList, pub data_root: Hash256, pub signature: AggregateSignature, } diff --git a/validator_client/src/attestation_service.rs b/validator_client/src/attestation_service.rs index 1c6b60addb6..653d829d685 100644 --- a/validator_client/src/attestation_service.rs +++ b/validator_client/src/attestation_service.rs @@ -15,8 +15,8 @@ use std::sync::Arc; use tokio::time::{sleep, sleep_until, Duration, Instant}; use tree_hash::TreeHash; use types::{ - AggregateSignature, Attestation, AttestationData, BitList, ChainSpec, CommitteeIndex, EthSpec, - Slot, + attestation::AttestationBase, AggregateSignature, Attestation, AttestationData, BitList, + ChainSpec, CommitteeIndex, EthSpec, Slot, }; /// Builds an `AttestationService`. @@ -378,11 +378,11 @@ impl AttestationService { return None; } - let mut attestation = Attestation { + let mut attestation = Attestation::Base(AttestationBase { aggregation_bits: BitList::with_capacity(duty.committee_length as usize).unwrap(), data: attestation_data.clone(), signature: AggregateSignature::infinity(), - }; + }); match self .validator_store @@ -610,10 +610,10 @@ impl AttestationService { log, "Successfully published attestation"; "aggregator" => signed_aggregate_and_proof.message.aggregator_index, - "signatures" => attestation.aggregation_bits.num_set_bits(), - "head_block" => format!("{:?}", attestation.data.beacon_block_root), - "committee_index" => attestation.data.index, - "slot" => attestation.data.slot.as_u64(), + "signatures" => attestation.num_set_aggregation_bits(), + "head_block" => format!("{:?}", attestation.data().beacon_block_root), + "committee_index" => attestation.data().index, + "slot" => attestation.data().slot.as_u64(), "type" => "aggregated", ); } @@ -626,8 +626,8 @@ impl AttestationService { "Failed to publish attestation"; "error" => %e, "aggregator" => signed_aggregate_and_proof.message.aggregator_index, - "committee_index" => attestation.data.index, - "slot" => attestation.data.slot.as_u64(), + "committee_index" => attestation.data().index, + "slot" => attestation.data().slot.as_u64(), "type" => "aggregated", ); } diff --git a/validator_client/src/validator_store.rs b/validator_client/src/validator_store.rs index 0a00dad9beb..7334c35bfc7 100644 --- a/validator_client/src/validator_store.rs +++ b/validator_client/src/validator_store.rs @@ -647,9 +647,9 @@ impl ValidatorStore { current_epoch: Epoch, ) -> Result<(), Error> { // Make sure the target epoch is not higher than the current epoch to avoid potential attacks. - if attestation.data.target.epoch > current_epoch { + if attestation.data().target.epoch > current_epoch { return Err(Error::GreaterThanCurrentEpoch { - epoch: attestation.data.target.epoch, + epoch: attestation.data().target.epoch, current_epoch, }); } @@ -658,7 +658,7 @@ impl ValidatorStore { let signing_method = self.doppelganger_checked_signing_method(validator_pubkey)?; // Checking for slashing conditions. - let signing_epoch = attestation.data.target.epoch; + let signing_epoch = attestation.data().target.epoch; let signing_context = self.signing_context(Domain::BeaconAttester, signing_epoch); let domain_hash = signing_context.domain_hash(&self.spec); let slashing_status = if signing_method @@ -666,7 +666,7 @@ impl ValidatorStore { { self.slashing_protection.check_and_insert_attestation( &validator_pubkey, - &attestation.data, + attestation.data(), domain_hash, ) } else { @@ -678,7 +678,7 @@ impl ValidatorStore { Ok(Safe::Valid) => { let signature = signing_method .get_signature::>( - SignableMessage::AttestationData(&attestation.data), + SignableMessage::AttestationData(attestation.data()), signing_context, &self.spec, &self.task_executor, @@ -720,7 +720,7 @@ impl ValidatorStore { crit!( self.log, "Not signing slashable attestation"; - "attestation" => format!("{:?}", attestation.data), + "attestation" => format!("{:?}", attestation.data()), "error" => format!("{:?}", e) ); metrics::inc_counter_vec( @@ -798,7 +798,7 @@ impl ValidatorStore { aggregate: Attestation, selection_proof: SelectionProof, ) -> Result, Error> { - let signing_epoch = aggregate.data.target.epoch; + let signing_epoch = aggregate.data().target.epoch; let signing_context = self.signing_context(Domain::AggregateAndProof, signing_epoch); let message = AggregateAndProof { From 864e00d7e440104b87d7e8cfa39ffef88c0751b0 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Wed, 24 Apr 2024 12:23:28 +0300 Subject: [PATCH 6/9] fix tests --- beacon_node/http_api/tests/tests.rs | 17 ++++++++-------- beacon_node/operation_pool/src/lib.rs | 10 +++++----- consensus/fork_choice/tests/tests.rs | 2 +- .../src/per_block_processing/tests.rs | 20 +++++++++---------- testing/web3signer_tests/src/lib.rs | 20 +++++++++---------- .../src/http_api/tests/keystores.rs | 10 +++++----- 6 files changed, 40 insertions(+), 39 deletions(-) diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index e4580e4ffdb..182261b3e6c 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -36,7 +36,7 @@ use tree_hash::TreeHash; use types::application_domain::ApplicationDomain; use types::{ AggregateSignature, BitList, Domain, EthSpec, ExecutionBlockHash, Hash256, Keypair, - MainnetEthSpec, RelativeEpoch, SelectionProof, SignedRoot, Slot, + MainnetEthSpec, RelativeEpoch, SelectionProof, SignedRoot, Slot, attestation::AttestationBase }; type E = MainnetEthSpec; @@ -1669,7 +1669,7 @@ impl ApiTester { let mut attestations = Vec::new(); for attestation in &self.attestations { let mut invalid_attestation = attestation.clone(); - invalid_attestation.data.slot += 1; + invalid_attestation.data_mut().slot += 1; // add both to ensure we only fail on invalid attestations attestations.push(attestation.clone()); @@ -3168,7 +3168,8 @@ impl ApiTester { .chain .produce_unaggregated_attestation(slot, index) .unwrap() - .data; + .data() + .clone(); assert_eq!(result, expected); } @@ -3188,8 +3189,8 @@ impl ApiTester { let result = self .client .get_validator_aggregate_attestation( - attestation.data.slot, - attestation.data.tree_hash_root(), + attestation.data().slot, + attestation.data().tree_hash_root(), ) .await .unwrap() @@ -3269,11 +3270,11 @@ impl ApiTester { .unwrap() .data; - let mut attestation = Attestation { + let mut attestation = Attestation::Base(AttestationBase { aggregation_bits: BitList::with_capacity(duty.committee_length as usize).unwrap(), data: attestation_data, signature: AggregateSignature::infinity(), - }; + }); attestation .sign( @@ -3312,7 +3313,7 @@ impl ApiTester { pub async fn test_get_validator_aggregate_and_proofs_invalid(mut self) -> Self { let mut aggregate = self.get_aggregate().await; - aggregate.message.aggregate.data.slot += 1; + aggregate.message.aggregate.data_mut().slot += 1; self.client .post_validator_aggregate_and_proof::(&[aggregate]) diff --git a/beacon_node/operation_pool/src/lib.rs b/beacon_node/operation_pool/src/lib.rs index 7cd40a4bb60..aa0ed22c5fa 100644 --- a/beacon_node/operation_pool/src/lib.rs +++ b/beacon_node/operation_pool/src/lib.rs @@ -913,7 +913,7 @@ mod release_tests { let att2_split = SplitAttestation::new(att2.clone(), att2_indices); assert_eq!( - att1.aggregation_bits.num_set_bits(), + att1.num_set_aggregation_bits(), earliest_attestation_validators( &att1_split.as_ref(), &state, @@ -927,8 +927,8 @@ mod release_tests { .unwrap() .current_epoch_attestations .push(PendingAttestation { - aggregation_bits: att1.aggregation_bits.clone(), - data: att1.data.clone(), + aggregation_bits: att1.aggregation_bits_base().unwrap().clone(), + data: att1.data().clone(), inclusion_delay: 0, proposer_index: 0, }) @@ -1007,7 +1007,7 @@ mod release_tests { let agg_att = &block_attestations[0]; assert_eq!( - agg_att.aggregation_bits.num_set_bits(), + agg_att.num_set_aggregation_bits(), spec.target_committee_size as usize ); @@ -1243,7 +1243,7 @@ mod release_tests { // All the best attestations should be signed by at least `big_step_size` (4) validators. for att in &best_attestations { - assert!(att.aggregation_bits.num_set_bits() >= big_step_size); + assert!(att.num_set_aggregation_bits() >= big_step_size); } } diff --git a/consensus/fork_choice/tests/tests.rs b/consensus/fork_choice/tests/tests.rs index 3153275fb73..d500c25c3e9 100644 --- a/consensus/fork_choice/tests/tests.rs +++ b/consensus/fork_choice/tests/tests.rs @@ -435,7 +435,7 @@ impl ForkChoiceTest { let validator_committee_index = 0; let validator_index = *head .beacon_state - .get_beacon_committee(current_slot, attestation.data.index) + .get_beacon_committee(current_slot, attestation.data().index) .expect("should get committees") .committee .get(validator_committee_index) diff --git a/consensus/state_processing/src/per_block_processing/tests.rs b/consensus/state_processing/src/per_block_processing/tests.rs index 2a2b67e30da..640297b44ae 100644 --- a/consensus/state_processing/src/per_block_processing/tests.rs +++ b/consensus/state_processing/src/per_block_processing/tests.rs @@ -394,7 +394,7 @@ async fn invalid_attestation_no_committee_for_index() { .deconstruct() .0; head_block.to_mut().body_mut().attestations_mut()[0] - .data + .data_mut() .index += 1; let mut ctxt = ConsensusContext::new(state.slot()); let result = process_operations::process_attestations( @@ -428,11 +428,11 @@ async fn invalid_attestation_wrong_justified_checkpoint() { .clone() .deconstruct() .0; - let old_justified_checkpoint = head_block.body().attestations()[0].data.source; + let old_justified_checkpoint = head_block.body().attestations()[0].data().source; let mut new_justified_checkpoint = old_justified_checkpoint; new_justified_checkpoint.epoch += Epoch::new(1); head_block.to_mut().body_mut().attestations_mut()[0] - .data + .data_mut() .source = new_justified_checkpoint; let mut ctxt = ConsensusContext::new(state.slot()); @@ -472,7 +472,7 @@ async fn invalid_attestation_bad_aggregation_bitfield_len() { .clone() .deconstruct() .0; - head_block.to_mut().body_mut().attestations_mut()[0].aggregation_bits = + *head_block.to_mut().body_mut().attestations_mut()[0].aggregation_bits_base_mut().unwrap() = Bitfield::with_capacity(spec.target_committee_size).unwrap(); let mut ctxt = ConsensusContext::new(state.slot()); @@ -506,7 +506,7 @@ async fn invalid_attestation_bad_signature() { .clone() .deconstruct() .0; - head_block.to_mut().body_mut().attestations_mut()[0].signature = AggregateSignature::empty(); + *head_block.to_mut().body_mut().attestations_mut()[0].signature_mut() = AggregateSignature::empty(); let mut ctxt = ConsensusContext::new(state.slot()); let result = process_operations::process_attestations( @@ -541,10 +541,10 @@ async fn invalid_attestation_included_too_early() { .clone() .deconstruct() .0; - let new_attesation_slot = head_block.body().attestations()[0].data.slot + let new_attesation_slot = head_block.body().attestations()[0].data().slot + Slot::new(MainnetEthSpec::slots_per_epoch()); head_block.to_mut().body_mut().attestations_mut()[0] - .data + .data_mut() .slot = new_attesation_slot; let mut ctxt = ConsensusContext::new(state.slot()); @@ -584,10 +584,10 @@ async fn invalid_attestation_included_too_late() { .clone() .deconstruct() .0; - let new_attesation_slot = head_block.body().attestations()[0].data.slot + let new_attesation_slot = head_block.body().attestations()[0].data().slot - Slot::new(MainnetEthSpec::slots_per_epoch()); head_block.to_mut().body_mut().attestations_mut()[0] - .data + .data_mut() .slot = new_attesation_slot; let mut ctxt = ConsensusContext::new(state.slot()); @@ -625,7 +625,7 @@ async fn invalid_attestation_target_epoch_slot_mismatch() { .deconstruct() .0; head_block.to_mut().body_mut().attestations_mut()[0] - .data + .data_mut() .target .epoch += Epoch::new(1); diff --git a/testing/web3signer_tests/src/lib.rs b/testing/web3signer_tests/src/lib.rs index 3090b4da556..50340d7983c 100644 --- a/testing/web3signer_tests/src/lib.rs +++ b/testing/web3signer_tests/src/lib.rs @@ -39,7 +39,7 @@ mod tests { use tempfile::{tempdir, TempDir}; use tokio::sync::OnceCell; use tokio::time::sleep; - use types::*; + use types::{*, attestation::AttestationBase}; use url::Url; use validator_client::{ initialized_validators::{ @@ -542,7 +542,7 @@ mod tests { /// Get a generic, arbitrary attestation for signing. fn get_attestation() -> Attestation { - Attestation { + Attestation::Base(AttestationBase { aggregation_bits: BitList::with_capacity(1).unwrap(), data: AttestationData { slot: <_>::default(), @@ -558,7 +558,7 @@ mod tests { }, }, signature: AggregateSignature::empty(), - } + }) } fn get_validator_registration(pubkey: PublicKeyBytes) -> ValidatorRegistrationData { @@ -771,28 +771,28 @@ mod tests { let first_attestation = || { let mut attestation = get_attestation(); - attestation.data.source.epoch = Epoch::new(1); - attestation.data.target.epoch = Epoch::new(4); + attestation.data_mut().source.epoch = Epoch::new(1); + attestation.data_mut().target.epoch = Epoch::new(4); attestation }; let double_vote_attestation = || { let mut attestation = first_attestation(); - attestation.data.beacon_block_root = Hash256::from_low_u64_be(1); + attestation.data_mut().beacon_block_root = Hash256::from_low_u64_be(1); attestation }; let surrounding_attestation = || { let mut attestation = first_attestation(); - attestation.data.source.epoch = Epoch::new(0); - attestation.data.target.epoch = Epoch::new(5); + attestation.data_mut().source.epoch = Epoch::new(0); + attestation.data_mut().target.epoch = Epoch::new(5); attestation }; let surrounded_attestation = || { let mut attestation = first_attestation(); - attestation.data.source.epoch = Epoch::new(2); - attestation.data.target.epoch = Epoch::new(3); + attestation.data_mut().source.epoch = Epoch::new(2); + attestation.data_mut().target.epoch = Epoch::new(3); attestation }; diff --git a/validator_client/src/http_api/tests/keystores.rs b/validator_client/src/http_api/tests/keystores.rs index fe58393bb8d..ae6b5f73d78 100644 --- a/validator_client/src/http_api/tests/keystores.rs +++ b/validator_client/src/http_api/tests/keystores.rs @@ -13,7 +13,7 @@ use rand::{rngs::SmallRng, Rng, SeedableRng}; use slashing_protection::interchange::{Interchange, InterchangeMetadata}; use std::{collections::HashMap, path::Path}; use tokio::runtime::Handle; -use types::Address; +use types::{Address, attestation::AttestationBase}; fn new_keystore(password: ZeroizeString) -> Keystore { let keypair = Keypair::random(); @@ -1094,7 +1094,7 @@ async fn generic_migration_test( // Sign attestations on VC1. for (validator_index, mut attestation) in first_vc_attestations { let public_key = keystore_pubkey(&keystores[validator_index]); - let current_epoch = attestation.data.target.epoch; + let current_epoch = attestation.data().target.epoch; tester1 .validator_store .sign_attestation(public_key, 0, &mut attestation, current_epoch) @@ -1170,7 +1170,7 @@ async fn generic_migration_test( // Sign attestations on the second VC. for (validator_index, mut attestation, should_succeed) in second_vc_attestations { let public_key = keystore_pubkey(&keystores[validator_index]); - let current_epoch = attestation.data.target.epoch; + let current_epoch = attestation.data().target.epoch; match tester2 .validator_store .sign_attestation(public_key, 0, &mut attestation, current_epoch) @@ -1236,7 +1236,7 @@ async fn delete_nonexistent_keystores() { } fn make_attestation(source_epoch: u64, target_epoch: u64) -> Attestation { - Attestation { + Attestation::Base(AttestationBase { aggregation_bits: BitList::with_capacity( ::MaxValidatorsPerCommittee::to_usize(), ) @@ -1253,7 +1253,7 @@ fn make_attestation(source_epoch: u64, target_epoch: u64) -> Attestation { ..AttestationData::default() }, signature: AggregateSignature::empty(), - } + }) } #[tokio::test] From 3680aebb90308b4618dcecd307af570d9cffc0c8 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Wed, 24 Apr 2024 21:39:28 +0300 Subject: [PATCH 7/9] indexed attestation --- .../src/attestation_verification.rs | 15 +- .../src/attestation_verification/batch.rs | 8 +- beacon_node/beacon_chain/src/beacon_chain.rs | 2 +- .../beacon_chain/src/observed_operations.rs | 6 +- beacon_node/beacon_chain/src/test_utils.rs | 38 ++--- .../beacon_chain/src/validator_monitor.rs | 20 ++- beacon_node/http_api/tests/tests.rs | 6 +- .../gossip_methods.rs | 6 +- consensus/fork_choice/src/fork_choice.rs | 39 +++--- consensus/fork_choice/tests/tests.rs | 22 +-- .../src/common/get_indexed_attestation.rs | 6 +- .../is_valid_indexed_attestation.rs | 6 +- .../process_operations.rs | 7 +- .../per_block_processing/signature_sets.rs | 20 +-- .../src/per_block_processing/tests.rs | 18 ++- .../verify_attester_slashing.rs | 7 +- .../state_processing/src/verify_operation.rs | 4 +- consensus/types/src/beacon_block.rs | 21 +-- consensus/types/src/chain_spec.rs | 13 -- consensus/types/src/eth_spec.rs | 13 -- consensus/types/src/indexed_attestation.rs | 131 ++++++++++++++++-- slasher/src/array.rs | 20 +-- slasher/src/attestation_queue.rs | 7 +- slasher/src/attester_record.rs | 8 +- slasher/src/config.rs | 3 +- slasher/src/database.rs | 6 +- slasher/src/slasher.rs | 6 +- slasher/src/test_utils.rs | 21 ++- testing/web3signer_tests/src/lib.rs | 2 +- .../src/http_api/tests/keystores.rs | 2 +- 30 files changed, 286 insertions(+), 197 deletions(-) diff --git a/beacon_node/beacon_chain/src/attestation_verification.rs b/beacon_node/beacon_chain/src/attestation_verification.rs index 873203b2b18..131932febba 100644 --- a/beacon_node/beacon_chain/src/attestation_verification.rs +++ b/beacon_node/beacon_chain/src/attestation_verification.rs @@ -329,7 +329,7 @@ pub trait VerifiedAttestation: Sized { // Inefficient default implementation. This is overridden for gossip verified attestations. fn into_attestation_and_indices(self) -> (Attestation, Vec) { let attestation = self.attestation().clone(); - let attesting_indices = self.indexed_attestation().attesting_indices.clone().into(); + let attesting_indices = self.indexed_attestation().attesting_indices_to_vec(); (attestation, attesting_indices) } } @@ -757,7 +757,7 @@ impl<'a, T: BeaconChainTypes> IndexedUnaggregatedAttestation<'a, T> { chain: &BeaconChain, ) -> Result<(u64, SubnetId), Error> { let expected_subnet_id = SubnetId::compute_subnet_for_attestation_data::( - &indexed_attestation.data, + indexed_attestation.data(), committees_per_slot, &chain.spec, ) @@ -774,8 +774,7 @@ impl<'a, T: BeaconChainTypes> IndexedUnaggregatedAttestation<'a, T> { }; let validator_index = *indexed_attestation - .attesting_indices - .first() + .attesting_indices_first() .ok_or(Error::NotExactlyOneAggregationBitSet(0))?; /* @@ -1095,11 +1094,11 @@ pub fn verify_attestation_signature( let fork = chain .spec - .fork_at_epoch(indexed_attestation.data.target.epoch); + .fork_at_epoch(indexed_attestation.data().target.epoch); let signature_set = indexed_attestation_signature_set_from_pubkeys( |validator_index| pubkey_cache.get(validator_index).map(Cow::Borrowed), - &indexed_attestation.signature, + indexed_attestation.signature(), indexed_attestation, &fork, chain.genesis_validators_root, @@ -1199,7 +1198,7 @@ pub fn verify_signed_aggregate_signatures( let fork = chain .spec - .fork_at_epoch(indexed_attestation.data.target.epoch); + .fork_at_epoch(indexed_attestation.data().target.epoch); let signature_sets = vec![ signed_aggregate_selection_proof_signature_set( @@ -1220,7 +1219,7 @@ pub fn verify_signed_aggregate_signatures( .map_err(BeaconChainError::SignatureSetError)?, indexed_attestation_signature_set_from_pubkeys( |validator_index| pubkey_cache.get(validator_index).map(Cow::Borrowed), - &indexed_attestation.signature, + indexed_attestation.signature(), indexed_attestation, &fork, chain.genesis_validators_root, diff --git a/beacon_node/beacon_chain/src/attestation_verification/batch.rs b/beacon_node/beacon_chain/src/attestation_verification/batch.rs index 6aec2bef68a..1ec752ff5c5 100644 --- a/beacon_node/beacon_chain/src/attestation_verification/batch.rs +++ b/beacon_node/beacon_chain/src/attestation_verification/batch.rs @@ -73,7 +73,7 @@ where let indexed_attestation = &indexed.indexed_attestation; let fork = chain .spec - .fork_at_epoch(indexed_attestation.data.target.epoch); + .fork_at_epoch(indexed_attestation.data().target.epoch); signature_sets.push( signed_aggregate_selection_proof_signature_set( @@ -98,7 +98,7 @@ where signature_sets.push( indexed_attestation_signature_set_from_pubkeys( |validator_index| pubkey_cache.get(validator_index).map(Cow::Borrowed), - &indexed_attestation.signature, + indexed_attestation.signature(), indexed_attestation, &fork, chain.genesis_validators_root, @@ -182,11 +182,11 @@ where let indexed_attestation = &partially_verified.indexed_attestation; let fork = chain .spec - .fork_at_epoch(indexed_attestation.data.target.epoch); + .fork_at_epoch(indexed_attestation.data().target.epoch); let signature_set = indexed_attestation_signature_set_from_pubkeys( |validator_index| pubkey_cache.get(validator_index).map(Cow::Borrowed), - &indexed_attestation.signature, + indexed_attestation.signature(), indexed_attestation, &fork, chain.genesis_validators_root, diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 2e40e27e479..0868989e692 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -3825,7 +3825,7 @@ impl BeaconChain { let mut observed_block_attesters = self.observed_block_attesters.write(); - for &validator_index in &indexed_attestation.attesting_indices { + for &validator_index in indexed_attestation.attesting_indices_iter() { if let Err(e) = observed_block_attesters .observe_validator(a.data().target.epoch, validator_index as usize) { diff --git a/beacon_node/beacon_chain/src/observed_operations.rs b/beacon_node/beacon_chain/src/observed_operations.rs index 04861fbe318..a61cc4f74cc 100644 --- a/beacon_node/beacon_chain/src/observed_operations.rs +++ b/beacon_node/beacon_chain/src/observed_operations.rs @@ -63,14 +63,12 @@ impl ObservableOperation for AttesterSlashing { fn observed_validators(&self) -> SmallVec<[u64; SMALL_VEC_SIZE]> { let attestation_1_indices = self .attestation_1 - .attesting_indices - .iter() + .attesting_indices_iter() .copied() .collect::>(); let attestation_2_indices = self .attestation_2 - .attesting_indices - .iter() + .attesting_indices_iter() .copied() .collect::>(); attestation_1_indices diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index 0983906cf14..4debbcde62e 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -62,6 +62,7 @@ use task_executor::TaskExecutor; use task_executor::{test_utils::TestRuntime, ShutdownReason}; use tree_hash::TreeHash; use types::attestation::AttestationBase; +use types::indexed_attestation::IndexedAttestationBase; use types::payload::BlockProductionVersion; pub use types::test_utils::generate_deterministic_keypairs; use types::test_utils::TestRandom; @@ -1497,7 +1498,7 @@ where ) -> AttesterSlashing { let fork = self.chain.canonical_head.cached_head().head_fork(); - let mut attestation_1 = IndexedAttestation { + let mut attestation_1 = IndexedAttestation::Base(IndexedAttestationBase { attesting_indices: VariableList::new(validator_indices).unwrap(), data: AttestationData { slot: Slot::new(0), @@ -1513,28 +1514,29 @@ where }, }, signature: AggregateSignature::infinity(), - }; + }); let mut attestation_2 = attestation_1.clone(); - attestation_2.data.index += 1; - attestation_2.data.source.epoch = source2.unwrap_or(Epoch::new(0)); - attestation_2.data.target.epoch = target2.unwrap_or(fork.epoch); + attestation_2.data_mut().index += 1; + attestation_2.data_mut().source.epoch = source2.unwrap_or(Epoch::new(0)); + attestation_2.data_mut().target.epoch = target2.unwrap_or(fork.epoch); for attestation in &mut [&mut attestation_1, &mut attestation_2] { - for &i in &attestation.attesting_indices { + // TODO(eip7549) we could explore iter mut here + for i in attestation.attesting_indices_to_vec() { let sk = &self.validator_keypairs[i as usize].sk; let genesis_validators_root = self.chain.genesis_validators_root; let domain = self.chain.spec.get_domain( - attestation.data.target.epoch, + attestation.data().target.epoch, Domain::BeaconAttester, &fork, genesis_validators_root, ); - let message = attestation.data.signing_root(domain); + let message = attestation.data().signing_root(domain); - attestation.signature.add_assign(&sk.sign(message)); + attestation.signature_mut().add_assign(&sk.sign(message)); } } @@ -1563,36 +1565,36 @@ where }, }; - let mut attestation_1 = IndexedAttestation { + let mut attestation_1 = IndexedAttestation::Base(IndexedAttestationBase { attesting_indices: VariableList::new(validator_indices_1).unwrap(), data: data.clone(), signature: AggregateSignature::infinity(), - }; + }); - let mut attestation_2 = IndexedAttestation { + let mut attestation_2 = IndexedAttestation::Base(IndexedAttestationBase { attesting_indices: VariableList::new(validator_indices_2).unwrap(), data, signature: AggregateSignature::infinity(), - }; + }); - attestation_2.data.index += 1; + attestation_2.data_mut().index += 1; let fork = self.chain.canonical_head.cached_head().head_fork(); for attestation in &mut [&mut attestation_1, &mut attestation_2] { - for &i in &attestation.attesting_indices { + for i in attestation.attesting_indices_to_vec() { let sk = &self.validator_keypairs[i as usize].sk; let genesis_validators_root = self.chain.genesis_validators_root; let domain = self.chain.spec.get_domain( - attestation.data.target.epoch, + attestation.data().target.epoch, Domain::BeaconAttester, &fork, genesis_validators_root, ); - let message = attestation.data.signing_root(domain); + let message = attestation.data().signing_root(domain); - attestation.signature.add_assign(&sk.sign(message)); + attestation.signature_mut().add_assign(&sk.sign(message)); } } diff --git a/beacon_node/beacon_chain/src/validator_monitor.rs b/beacon_node/beacon_chain/src/validator_monitor.rs index 8da3f680a5c..70f86ac138e 100644 --- a/beacon_node/beacon_chain/src/validator_monitor.rs +++ b/beacon_node/beacon_chain/src/validator_monitor.rs @@ -1233,7 +1233,7 @@ impl ValidatorMonitor { indexed_attestation: &IndexedAttestation, slot_clock: &S, ) { - let data = &indexed_attestation.data; + let data = indexed_attestation.data(); let epoch = data.slot.epoch(E::slots_per_epoch()); let delay = get_message_delay_ms( seen_timestamp, @@ -1242,7 +1242,7 @@ impl ValidatorMonitor { slot_clock, ); - indexed_attestation.attesting_indices.iter().for_each(|i| { + indexed_attestation.attesting_indices_iter().for_each(|i| { if let Some(validator) = self.get_validator(*i) { let id = &validator.id; @@ -1321,7 +1321,7 @@ impl ValidatorMonitor { indexed_attestation: &IndexedAttestation, slot_clock: &S, ) { - let data = &indexed_attestation.data; + let data = indexed_attestation.data(); let epoch = data.slot.epoch(E::slots_per_epoch()); let delay = get_message_delay_ms( seen_timestamp, @@ -1365,7 +1365,7 @@ impl ValidatorMonitor { }); } - indexed_attestation.attesting_indices.iter().for_each(|i| { + indexed_attestation.attesting_indices_iter().for_each(|i| { if let Some(validator) = self.get_validator(*i) { let id = &validator.id; @@ -1414,7 +1414,7 @@ impl ValidatorMonitor { parent_slot: Slot, spec: &ChainSpec, ) { - let data = &indexed_attestation.data; + let data = indexed_attestation.data(); // Best effort inclusion distance which ignores skip slots between the parent // and the current block. Skipped slots between the attestation slot and the parent // slot are still counted for simplicity's sake. @@ -1423,7 +1423,7 @@ impl ValidatorMonitor { let delay = inclusion_distance - spec.min_attestation_inclusion_delay; let epoch = data.slot.epoch(E::slots_per_epoch()); - indexed_attestation.attesting_indices.iter().for_each(|i| { + indexed_attestation.attesting_indices_iter().for_each(|i| { if let Some(validator) = self.get_validator(*i) { let id = &validator.id; @@ -1798,18 +1798,16 @@ impl ValidatorMonitor { } fn register_attester_slashing(&self, src: &str, slashing: &AttesterSlashing) { - let data = &slashing.attestation_1.data; + let data = slashing.attestation_1.data(); let attestation_1_indices: HashSet = slashing .attestation_1 - .attesting_indices - .iter() + .attesting_indices_iter() .copied() .collect(); slashing .attestation_2 - .attesting_indices - .iter() + .attesting_indices_iter() .filter(|index| attestation_1_indices.contains(index)) .filter_map(|index| self.get_validator(*index)) .for_each(|validator| { diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index 182261b3e6c..15a340d49c1 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -35,8 +35,8 @@ use tokio::time::Duration; use tree_hash::TreeHash; use types::application_domain::ApplicationDomain; use types::{ - AggregateSignature, BitList, Domain, EthSpec, ExecutionBlockHash, Hash256, Keypair, - MainnetEthSpec, RelativeEpoch, SelectionProof, SignedRoot, Slot, attestation::AttestationBase + attestation::AttestationBase, AggregateSignature, BitList, Domain, EthSpec, ExecutionBlockHash, + Hash256, Keypair, MainnetEthSpec, RelativeEpoch, SelectionProof, SignedRoot, Slot, }; type E = MainnetEthSpec; @@ -1793,7 +1793,7 @@ impl ApiTester { pub async fn test_post_beacon_pool_attester_slashings_invalid(mut self) -> Self { let mut slashing = self.attester_slashing.clone(); - slashing.attestation_1.data.slot += 1; + slashing.attestation_1.data_mut().slot += 1; self.client .post_beacon_pool_attester_slashings(&slashing) diff --git a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs index 3897d0a0841..772c2efc944 100644 --- a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs @@ -72,7 +72,7 @@ impl VerifiedAttestation for VerifiedUnaggregate { fn into_attestation_and_indices(self) -> (Attestation, Vec) { let attestation = *self.attestation; - let attesting_indices = self.indexed_attestation.attesting_indices.into(); + let attesting_indices = self.indexed_attestation.attesting_indices_to_vec(); (attestation, attesting_indices) } } @@ -106,7 +106,7 @@ impl VerifiedAttestation for VerifiedAggregate { /// Efficient clone-free implementation that moves out of the `Box`. fn into_attestation_and_indices(self) -> (Attestation, Vec) { let attestation = self.signed_aggregate.message.aggregate; - let attesting_indices = self.indexed_attestation.attesting_indices.into(); + let attesting_indices = self.indexed_attestation.attesting_indices_to_vec(); (attestation, attesting_indices) } } @@ -309,7 +309,7 @@ impl NetworkBeaconProcessor { match result { Ok(verified_attestation) => { let indexed_attestation = &verified_attestation.indexed_attestation; - let beacon_block_root = indexed_attestation.data.beacon_block_root; + let beacon_block_root = indexed_attestation.data().beacon_block_root; // Register the attestation with any monitored validators. self.chain diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index 6e3f6717ede..f7836fb2fb4 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -241,10 +241,10 @@ pub struct QueuedAttestation { impl From<&IndexedAttestation> for QueuedAttestation { fn from(a: &IndexedAttestation) -> Self { Self { - slot: a.data.slot, - attesting_indices: a.attesting_indices[..].to_vec(), - block_root: a.data.beacon_block_root, - target_epoch: a.data.target.epoch, + slot: a.data().slot, + attesting_indices: a.attesting_indices_to_vec(), + block_root: a.data().beacon_block_root, + target_epoch: a.data().target.epoch, } } } @@ -948,20 +948,20 @@ where // // This is not in the specification, however it should be transparent to other nodes. We // return early here to avoid wasting precious resources verifying the rest of it. - if indexed_attestation.attesting_indices.is_empty() { + if indexed_attestation.attesting_indices_is_empty() { return Err(InvalidAttestation::EmptyAggregationBitfield); } - let target = indexed_attestation.data.target; + let target = indexed_attestation.data().target; if matches!(is_from_block, AttestationFromBlock::False) { self.validate_target_epoch_against_current_time(target.epoch)?; } - if target.epoch != indexed_attestation.data.slot.epoch(E::slots_per_epoch()) { + if target.epoch != indexed_attestation.data().slot.epoch(E::slots_per_epoch()) { return Err(InvalidAttestation::BadTargetEpoch { target: target.epoch, - slot: indexed_attestation.data.slot, + slot: indexed_attestation.data().slot, }); } @@ -983,9 +983,9 @@ where // attestation and do not delay consideration for later. let block = self .proto_array - .get_block(&indexed_attestation.data.beacon_block_root) + .get_block(&indexed_attestation.data().beacon_block_root) .ok_or(InvalidAttestation::UnknownHeadBlock { - beacon_block_root: indexed_attestation.data.beacon_block_root, + beacon_block_root: indexed_attestation.data().beacon_block_root, })?; // If an attestation points to a block that is from an earlier slot than the attestation, @@ -993,7 +993,7 @@ where // is from a prior epoch to the attestation, then the target root must be equal to the root // of the block that is being attested to. let expected_target = if target.epoch > block.slot.epoch(E::slots_per_epoch()) { - indexed_attestation.data.beacon_block_root + indexed_attestation.data().beacon_block_root } else { block.target_root }; @@ -1007,10 +1007,10 @@ where // Attestations must not be for blocks in the future. If this is the case, the attestation // should not be considered. - if block.slot > indexed_attestation.data.slot { + if block.slot > indexed_attestation.data().slot { return Err(InvalidAttestation::AttestsToFutureBlock { block: block.slot, - attestation: indexed_attestation.data.slot, + attestation: indexed_attestation.data().slot, }); } @@ -1055,18 +1055,18 @@ where // (1) becomes weird once we hit finality and fork choice drops the genesis block. (2) is // fine because votes to the genesis block are not useful; all validators implicitly attest // to genesis just by being present in the chain. - if attestation.data.beacon_block_root == Hash256::zero() { + if attestation.data().beacon_block_root == Hash256::zero() { return Ok(()); } self.validate_on_attestation(attestation, is_from_block)?; - if attestation.data.slot < self.fc_store.get_current_slot() { - for validator_index in attestation.attesting_indices.iter() { + if attestation.data().slot < self.fc_store.get_current_slot() { + for validator_index in attestation.attesting_indices_iter() { self.proto_array.process_attestation( *validator_index as usize, - attestation.data.beacon_block_root, - attestation.data.target.epoch, + attestation.data().beacon_block_root, + attestation.data().target.epoch, )?; } } else { @@ -1088,8 +1088,7 @@ where /// We assume that the attester slashing provided to this function has already been verified. pub fn on_attester_slashing(&mut self, slashing: &AttesterSlashing) { let attesting_indices_set = |att: &IndexedAttestation| { - att.attesting_indices - .iter() + att.attesting_indices_iter() .copied() .collect::>() }; diff --git a/consensus/fork_choice/tests/tests.rs b/consensus/fork_choice/tests/tests.rs index d500c25c3e9..63bf3b51bd0 100644 --- a/consensus/fork_choice/tests/tests.rs +++ b/consensus/fork_choice/tests/tests.rs @@ -831,7 +831,7 @@ async fn invalid_attestation_empty_bitfield() { .apply_attestation_to_chain( MutationDelay::NoDelay, |attestation, _| { - attestation.attesting_indices = vec![].into(); + *attestation.attesting_indices_base_mut().unwrap() = vec![].into(); }, |result| { assert_invalid_attestation!(result, InvalidAttestation::EmptyAggregationBitfield) @@ -853,7 +853,7 @@ async fn invalid_attestation_future_epoch() { .apply_attestation_to_chain( MutationDelay::NoDelay, |attestation, _| { - attestation.data.target.epoch = Epoch::new(2); + attestation.data_mut().target.epoch = Epoch::new(2); }, |result| { assert_invalid_attestation!( @@ -879,7 +879,7 @@ async fn invalid_attestation_past_epoch() { .apply_attestation_to_chain( MutationDelay::NoDelay, |attestation, _| { - attestation.data.target.epoch = Epoch::new(0); + attestation.data_mut().target.epoch = Epoch::new(0); }, |result| { assert_invalid_attestation!( @@ -903,7 +903,7 @@ async fn invalid_attestation_target_epoch() { .apply_attestation_to_chain( MutationDelay::NoDelay, |attestation, _| { - attestation.data.slot = Slot::new(1); + attestation.data_mut().slot = Slot::new(1); }, |result| { assert_invalid_attestation!( @@ -929,7 +929,7 @@ async fn invalid_attestation_unknown_target_root() { .apply_attestation_to_chain( MutationDelay::NoDelay, |attestation, _| { - attestation.data.target.root = junk; + attestation.data_mut().target.root = junk; }, |result| { assert_invalid_attestation!( @@ -955,7 +955,7 @@ async fn invalid_attestation_unknown_beacon_block_root() { .apply_attestation_to_chain( MutationDelay::NoDelay, |attestation, _| { - attestation.data.beacon_block_root = junk; + attestation.data_mut().beacon_block_root = junk; }, |result| { assert_invalid_attestation!( @@ -979,7 +979,7 @@ async fn invalid_attestation_future_block() { .apply_attestation_to_chain( MutationDelay::Blocks(1), |attestation, chain| { - attestation.data.beacon_block_root = chain + attestation.data_mut().beacon_block_root = chain .block_at_slot(chain.slot().unwrap(), WhenSlotSkipped::Prev) .unwrap() .unwrap() @@ -1010,13 +1010,13 @@ async fn invalid_attestation_inconsistent_ffg_vote() { .apply_attestation_to_chain( MutationDelay::NoDelay, |attestation, chain| { - attestation.data.target.root = chain + attestation.data_mut().target.root = chain .block_at_slot(Slot::new(1), WhenSlotSkipped::Prev) .unwrap() .unwrap() .canonical_root(); - *attestation_opt.lock().unwrap() = Some(attestation.data.target.root); + *attestation_opt.lock().unwrap() = Some(attestation.data().target.root); *local_opt.lock().unwrap() = Some( chain .block_at_slot(Slot::new(0), WhenSlotSkipped::Prev) @@ -1069,8 +1069,8 @@ async fn valid_attestation_skip_across_epoch() { MutationDelay::NoDelay, |attestation, _chain| { assert_eq!( - attestation.data.target.root, - attestation.data.beacon_block_root + attestation.data().target.root, + attestation.data().beacon_block_root ) }, |result| result.unwrap(), diff --git a/consensus/state_processing/src/common/get_indexed_attestation.rs b/consensus/state_processing/src/common/get_indexed_attestation.rs index 7643a38c928..b68d5ab61e8 100644 --- a/consensus/state_processing/src/common/get_indexed_attestation.rs +++ b/consensus/state_processing/src/common/get_indexed_attestation.rs @@ -1,6 +1,6 @@ use super::get_attesting_indices; use crate::per_block_processing::errors::{AttestationInvalid as Invalid, BlockOperationError}; -use types::*; +use types::{indexed_attestation::IndexedAttestationBase, *}; type Result = std::result::Result>; @@ -17,9 +17,9 @@ pub fn get_indexed_attestation( Attestation::Electra(_) => todo!(), }; - Ok(IndexedAttestation { + Ok(IndexedAttestation::Base(IndexedAttestationBase { attesting_indices: VariableList::new(attesting_indices)?, data: attestation.data().clone(), signature: attestation.signature().clone(), - }) + })) } diff --git a/consensus/state_processing/src/per_block_processing/is_valid_indexed_attestation.rs b/consensus/state_processing/src/per_block_processing/is_valid_indexed_attestation.rs index baccd1dbbd2..7c7c9e474ac 100644 --- a/consensus/state_processing/src/per_block_processing/is_valid_indexed_attestation.rs +++ b/consensus/state_processing/src/per_block_processing/is_valid_indexed_attestation.rs @@ -17,7 +17,7 @@ pub fn is_valid_indexed_attestation( verify_signatures: VerifySignatures, spec: &ChainSpec, ) -> Result<()> { - let indices = &indexed_attestation.attesting_indices; + let indices = indexed_attestation.attesting_indices_to_vec(); // Verify that indices aren't empty verify!(!indices.is_empty(), Invalid::IndicesEmpty); @@ -36,14 +36,14 @@ pub fn is_valid_indexed_attestation( })?; Ok(()) }; - check_sorted(indices)?; + check_sorted(&indices)?; if verify_signatures.is_true() { verify!( indexed_attestation_signature_set( state, |i| get_pubkey_from_state(state, i), - &indexed_attestation.signature, + indexed_attestation.signature(), indexed_attestation, spec )? diff --git a/consensus/state_processing/src/per_block_processing/process_operations.rs b/consensus/state_processing/src/per_block_processing/process_operations.rs index 47037c2f9b1..48b2676c157 100644 --- a/consensus/state_processing/src/per_block_processing/process_operations.rs +++ b/consensus/state_processing/src/per_block_processing/process_operations.rs @@ -137,15 +137,14 @@ pub mod altair_deneb { let previous_epoch = ctxt.previous_epoch; let current_epoch = ctxt.current_epoch; - let attesting_indices = &verify_attestation_for_block_inclusion( + let indexed_att = verify_attestation_for_block_inclusion( state, attestation, ctxt, verify_signatures, spec, ) - .map_err(|e| e.into_with_index(att_index))? - .attesting_indices; + .map_err(|e| e.into_with_index(att_index))?; // Matching roots, participation flag indices let data = attestation.data(); @@ -155,7 +154,7 @@ pub mod altair_deneb { // Update epoch participation flags. let mut proposer_reward_numerator = 0; - for index in attesting_indices { + for index in indexed_att.attesting_indices_iter() { let index = *index as usize; let validator_effective_balance = state.epoch_cache().get_effective_balance(index)?; diff --git a/consensus/state_processing/src/per_block_processing/signature_sets.rs b/consensus/state_processing/src/per_block_processing/signature_sets.rs index 92b6ed99449..19351a2c2f8 100644 --- a/consensus/state_processing/src/per_block_processing/signature_sets.rs +++ b/consensus/state_processing/src/per_block_processing/signature_sets.rs @@ -279,21 +279,21 @@ where E: EthSpec, F: Fn(usize) -> Option>, { - let mut pubkeys = Vec::with_capacity(indexed_attestation.attesting_indices.len()); - for &validator_idx in &indexed_attestation.attesting_indices { + let mut pubkeys = Vec::with_capacity(indexed_attestation.attesting_indices_len()); + for &validator_idx in indexed_attestation.attesting_indices_iter() { pubkeys.push( get_pubkey(validator_idx as usize).ok_or(Error::ValidatorUnknown(validator_idx))?, ); } let domain = spec.get_domain( - indexed_attestation.data.target.epoch, + indexed_attestation.data().target.epoch, Domain::BeaconAttester, &state.fork(), state.genesis_validators_root(), ); - let message = indexed_attestation.data.signing_root(domain); + let message = indexed_attestation.data().signing_root(domain); Ok(SignatureSet::multiple_pubkeys(signature, pubkeys, message)) } @@ -312,21 +312,21 @@ where E: EthSpec, F: Fn(usize) -> Option>, { - let mut pubkeys = Vec::with_capacity(indexed_attestation.attesting_indices.len()); - for &validator_idx in &indexed_attestation.attesting_indices { + let mut pubkeys = Vec::with_capacity(indexed_attestation.attesting_indices_len()); + for &validator_idx in indexed_attestation.attesting_indices_iter() { pubkeys.push( get_pubkey(validator_idx as usize).ok_or(Error::ValidatorUnknown(validator_idx))?, ); } let domain = spec.get_domain( - indexed_attestation.data.target.epoch, + indexed_attestation.data().target.epoch, Domain::BeaconAttester, fork, genesis_validators_root, ); - let message = indexed_attestation.data.signing_root(domain); + let message = indexed_attestation.data().signing_root(domain); Ok(SignatureSet::multiple_pubkeys(signature, pubkeys, message)) } @@ -346,14 +346,14 @@ where indexed_attestation_signature_set( state, get_pubkey.clone(), - &attester_slashing.attestation_1.signature, + attester_slashing.attestation_1.signature(), &attester_slashing.attestation_1, spec, )?, indexed_attestation_signature_set( state, get_pubkey, - &attester_slashing.attestation_2.signature, + attester_slashing.attestation_2.signature(), &attester_slashing.attestation_2, spec, )?, diff --git a/consensus/state_processing/src/per_block_processing/tests.rs b/consensus/state_processing/src/per_block_processing/tests.rs index 640297b44ae..781f9047a0b 100644 --- a/consensus/state_processing/src/per_block_processing/tests.rs +++ b/consensus/state_processing/src/per_block_processing/tests.rs @@ -472,8 +472,9 @@ async fn invalid_attestation_bad_aggregation_bitfield_len() { .clone() .deconstruct() .0; - *head_block.to_mut().body_mut().attestations_mut()[0].aggregation_bits_base_mut().unwrap() = - Bitfield::with_capacity(spec.target_committee_size).unwrap(); + *head_block.to_mut().body_mut().attestations_mut()[0] + .aggregation_bits_base_mut() + .unwrap() = Bitfield::with_capacity(spec.target_committee_size).unwrap(); let mut ctxt = ConsensusContext::new(state.slot()); let result = process_operations::process_attestations( @@ -506,7 +507,8 @@ async fn invalid_attestation_bad_signature() { .clone() .deconstruct() .0; - *head_block.to_mut().body_mut().attestations_mut()[0].signature_mut() = AggregateSignature::empty(); + *head_block.to_mut().body_mut().attestations_mut()[0].signature_mut() = + AggregateSignature::empty(); let mut ctxt = ConsensusContext::new(state.slot()); let result = process_operations::process_attestations( @@ -704,7 +706,10 @@ async fn invalid_attester_slashing_1_invalid() { let harness = get_harness::(EPOCH_OFFSET, VALIDATOR_COUNT).await; let mut attester_slashing = harness.make_attester_slashing(vec![1, 2]); - attester_slashing.attestation_1.attesting_indices = VariableList::from(vec![2, 1]); + *attester_slashing + .attestation_1 + .attesting_indices_base_mut() + .unwrap() = VariableList::from(vec![2, 1]); let mut state = harness.get_current_state(); let mut ctxt = ConsensusContext::new(state.slot()); @@ -735,7 +740,10 @@ async fn invalid_attester_slashing_2_invalid() { let harness = get_harness::(EPOCH_OFFSET, VALIDATOR_COUNT).await; let mut attester_slashing = harness.make_attester_slashing(vec![1, 2]); - attester_slashing.attestation_2.attesting_indices = VariableList::from(vec![2, 1]); + *attester_slashing + .attestation_2 + .attesting_indices_base_mut() + .unwrap() = VariableList::from(vec![2, 1]); let mut state = harness.get_current_state(); let mut ctxt = ConsensusContext::new(state.slot()); diff --git a/consensus/state_processing/src/per_block_processing/verify_attester_slashing.rs b/consensus/state_processing/src/per_block_processing/verify_attester_slashing.rs index 0cb215fe93f..7fb784a968e 100644 --- a/consensus/state_processing/src/per_block_processing/verify_attester_slashing.rs +++ b/consensus/state_processing/src/per_block_processing/verify_attester_slashing.rs @@ -66,13 +66,12 @@ where let attestation_2 = &attester_slashing.attestation_2; let attesting_indices_1 = attestation_1 - .attesting_indices - .iter() + .attesting_indices_iter() .cloned() .collect::>(); + let attesting_indices_2 = attestation_2 - .attesting_indices - .iter() + .attesting_indices_iter() .cloned() .collect::>(); diff --git a/consensus/state_processing/src/verify_operation.rs b/consensus/state_processing/src/verify_operation.rs index b3924cd9732..4a2e5e22c56 100644 --- a/consensus/state_processing/src/verify_operation.rs +++ b/consensus/state_processing/src/verify_operation.rs @@ -159,8 +159,8 @@ impl VerifyOperation for AttesterSlashing { #[allow(clippy::arithmetic_side_effects)] fn verification_epochs(&self) -> SmallVec<[Epoch; MAX_FORKS_VERIFIED_AGAINST]> { smallvec![ - self.attestation_1.data.target.epoch, - self.attestation_2.data.target.epoch + self.attestation_1.data().target.epoch, + self.attestation_2.data().target.epoch ] } } diff --git a/consensus/types/src/beacon_block.rs b/consensus/types/src/beacon_block.rs index cf8c45e07f4..dd1a12abba1 100644 --- a/consensus/types/src/beacon_block.rs +++ b/consensus/types/src/beacon_block.rs @@ -11,6 +11,8 @@ use test_random_derive::TestRandom; use tree_hash::TreeHash; use tree_hash_derive::TreeHash; +use self::indexed_attestation::IndexedAttestationBase; + /// A block of the `BeaconChain`. #[superstruct( variants(Base, Altair, Merge, Capella, Deneb, Electra), @@ -325,15 +327,16 @@ impl> BeaconBlockBase { message: header, signature: Signature::empty(), }; - let indexed_attestation: IndexedAttestation = IndexedAttestation { - attesting_indices: VariableList::new(vec![ - 0_u64; - E::MaxValidatorsPerCommittee::to_usize() - ]) - .unwrap(), - data: AttestationData::default(), - signature: AggregateSignature::empty(), - }; + let indexed_attestation: IndexedAttestation = + IndexedAttestation::Base(IndexedAttestationBase { + attesting_indices: VariableList::new(vec![ + 0_u64; + E::MaxValidatorsPerCommittee::to_usize() + ]) + .unwrap(), + data: AttestationData::default(), + signature: AggregateSignature::empty(), + }); let deposit_data = DepositData { pubkey: PublicKeyBytes::empty(), diff --git a/consensus/types/src/chain_spec.rs b/consensus/types/src/chain_spec.rs index 96b19731a86..e4f27d6873c 100644 --- a/consensus/types/src/chain_spec.rs +++ b/consensus/types/src/chain_spec.rs @@ -229,7 +229,6 @@ pub struct ChainSpec { pub max_blocks_by_root_request: usize, pub max_blocks_by_root_request_deneb: usize, pub max_blobs_by_root_request: usize, - pub max_data_columns_by_range_request: usize, /* * Application params @@ -722,11 +721,6 @@ impl ChainSpec { deneb_fork_version: [0x04, 0x00, 0x00, 0x00], deneb_fork_epoch: Some(Epoch::new(269568)), - /* - * DAS params - */ - max_data_columns_by_range_request: 128, - /* * Electra hard fork params */ @@ -853,8 +847,6 @@ impl ChainSpec { // Deneb deneb_fork_version: [0x04, 0x00, 0x00, 0x01], deneb_fork_epoch: None, - // DAS - max_data_columns_by_range_request: 128, // Electra electra_fork_version: [0x05, 0x00, 0x00, 0x01], electra_fork_epoch: None, @@ -1029,11 +1021,6 @@ impl ChainSpec { deneb_fork_version: [0x04, 0x00, 0x00, 0x64], deneb_fork_epoch: Some(Epoch::new(889856)), - /* - * DAS params - */ - max_data_columns_by_range_request: 128, - /* * Electra hard fork params */ diff --git a/consensus/types/src/eth_spec.rs b/consensus/types/src/eth_spec.rs index 2f27209b9fb..f9031f98908 100644 --- a/consensus/types/src/eth_spec.rs +++ b/consensus/types/src/eth_spec.rs @@ -137,11 +137,6 @@ pub trait EthSpec: /// Must be set to `BytesPerFieldElement * FieldElementsPerBlob`. type BytesPerBlob: Unsigned + Clone + Sync + Send + Debug + PartialEq; - /* - * New for DAS - */ - type MaxDataColumnsPerBlock: Unsigned + Clone + Sync + Send + Debug + PartialEq + Unpin; - /* * New in Electra */ @@ -279,11 +274,6 @@ pub trait EthSpec: Self::MaxBlobsPerBlock::to_usize() } - /// Returns the `MAX_DATA_COLUMNS_PER_BLOCK` constant for this specification. - fn max_data_columns_per_block() -> usize { - Self::MaxDataColumnsPerBlock::to_usize() - } - /// Returns the `MAX_BLOB_COMMITMENTS_PER_BLOCK` constant for this specification. fn max_blob_commitments_per_block() -> usize { Self::MaxBlobCommitmentsPerBlock::to_usize() @@ -385,7 +375,6 @@ impl EthSpec for MainnetEthSpec { type MinGasLimit = U5000; type MaxExtraDataBytes = U32; type MaxBlobsPerBlock = U6; - type MaxDataColumnsPerBlock = U64; type MaxBlobCommitmentsPerBlock = U4096; type BytesPerFieldElement = U32; type FieldElementsPerBlob = U4096; @@ -457,7 +446,6 @@ impl EthSpec for MinimalEthSpec { MaxExtraDataBytes, MaxBlsToExecutionChanges, MaxBlobsPerBlock, - MaxDataColumnsPerBlock, BytesPerFieldElement, PendingBalanceDepositsLimit, PendingPartialWithdrawalsLimit, @@ -515,7 +503,6 @@ impl EthSpec for GnosisEthSpec { type MaxBlsToExecutionChanges = U16; type MaxWithdrawalsPerPayload = U8; type MaxBlobsPerBlock = U6; - type MaxDataColumnsPerBlock = U64; type MaxBlobCommitmentsPerBlock = U4096; type FieldElementsPerBlob = U4096; type BytesPerFieldElement = U32; diff --git a/consensus/types/src/indexed_attestation.rs b/consensus/types/src/indexed_attestation.rs index fd3bd99a3e4..4477b236fad 100644 --- a/consensus/types/src/indexed_attestation.rs +++ b/consensus/types/src/indexed_attestation.rs @@ -1,9 +1,13 @@ use crate::{test_utils::TestRandom, AggregateSignature, AttestationData, EthSpec, VariableList}; +use core::slice::Iter; use derivative::Derivative; +use rand::RngCore; use serde::{Deserialize, Serialize}; +use ssz::Decode; use ssz::Encode; use ssz_derive::{Decode, Encode}; use std::hash::{Hash, Hasher}; +use superstruct::superstruct; use test_random_derive::TestRandom; use tree_hash_derive::TreeHash; @@ -12,23 +16,48 @@ use tree_hash_derive::TreeHash; /// To be included in an `AttesterSlashing`. /// /// Spec v0.12.1 +#[superstruct( + variants(Base, Electra), + variant_attributes( + derive( + Debug, + Clone, + Serialize, + Deserialize, + Decode, + Encode, + TestRandom, + Derivative, + arbitrary::Arbitrary, + TreeHash, + ), + derivative(PartialEq, Hash(bound = "E: EthSpec")), + serde(bound = "E: EthSpec", deny_unknown_fields), + arbitrary(bound = "E: EthSpec"), + ) +)] #[derive( - Derivative, Debug, Clone, Serialize, - Deserialize, - Encode, - Decode, TreeHash, - TestRandom, + Encode, + Derivative, + Deserialize, arbitrary::Arbitrary, + PartialEq, )] -#[derivative(PartialEq, Eq)] // to satisfy Clippy's lint about `Hash` -#[serde(bound = "E: EthSpec")] +#[serde(untagged)] +#[tree_hash(enum_behaviour = "transparent")] +#[ssz(enum_behaviour = "transparent")] +#[serde(bound = "E: EthSpec", deny_unknown_fields)] #[arbitrary(bound = "E: EthSpec")] pub struct IndexedAttestation { /// Lists validator registry indices, not committee indices. + #[superstruct(only(Base), partial_getter(rename = "attesting_indices_base"))] + #[serde(with = "quoted_variable_list_u64")] + pub attesting_indices: VariableList, + #[superstruct(only(Electra), partial_getter(rename = "attesting_indices_electra"))] #[serde(with = "quoted_variable_list_u64")] pub attesting_indices: VariableList, pub data: AttestationData, @@ -40,15 +69,84 @@ impl IndexedAttestation { /// /// Spec v0.12.1 pub fn is_double_vote(&self, other: &Self) -> bool { - self.data.target.epoch == other.data.target.epoch && self.data != other.data + self.data().target.epoch == other.data().target.epoch && self.data() != other.data() } /// Check if ``attestation_data_1`` surrounds ``attestation_data_2``. /// /// Spec v0.12.1 pub fn is_surround_vote(&self, other: &Self) -> bool { - self.data.source.epoch < other.data.source.epoch - && other.data.target.epoch < self.data.target.epoch + self.data().source.epoch < other.data().source.epoch + && other.data().target.epoch < self.data().target.epoch + } + + pub fn attesting_indices_len(&self) -> usize { + match self { + IndexedAttestation::Base(att) => att.attesting_indices.len(), + IndexedAttestation::Electra(att) => att.attesting_indices.len(), + } + } + + pub fn attesting_indices_to_vec(&self) -> Vec { + match self { + IndexedAttestation::Base(att) => att.attesting_indices.to_vec(), + IndexedAttestation::Electra(att) => att.attesting_indices.to_vec(), + } + } + + pub fn attesting_indices_is_empty(&self) -> bool { + match self { + IndexedAttestation::Base(att) => att.attesting_indices.is_empty(), + IndexedAttestation::Electra(att) => att.attesting_indices.is_empty(), + } + } + + pub fn attesting_indices_iter(&self) -> Iter<'_, u64> { + match self { + IndexedAttestation::Base(att) => att.attesting_indices.iter(), + IndexedAttestation::Electra(att) => att.attesting_indices.iter(), + } + } + + pub fn attesting_indices_first(&self) -> Option<&u64> { + match self { + IndexedAttestation::Base(att) => att.attesting_indices.first(), + IndexedAttestation::Electra(att) => att.attesting_indices.first(), + } + } +} + +impl Decode for IndexedAttestation { + fn is_ssz_fixed_len() -> bool { + false + } + + fn from_ssz_bytes(bytes: &[u8]) -> Result { + if let Ok(result) = IndexedAttestationBase::from_ssz_bytes(bytes) { + return Ok(IndexedAttestation::Base(result)); + } + + if let Ok(result) = IndexedAttestationElectra::from_ssz_bytes(bytes) { + return Ok(IndexedAttestation::Electra(result)); + } + + Err(ssz::DecodeError::BytesInvalid(String::from( + "bytes not valid for any fork variant", + ))) + } +} + +impl TestRandom for IndexedAttestation { + fn random_for_test(rng: &mut impl RngCore) -> Self { + let attesting_indices = VariableList::random_for_test(rng); + let data = AttestationData::random_for_test(rng); + let signature = AggregateSignature::random_for_test(rng); + + Self::Base(IndexedAttestationBase { + attesting_indices, + data, + signature, + }) } } @@ -59,9 +157,12 @@ impl IndexedAttestation { /// Used in the operation pool. impl Hash for IndexedAttestation { fn hash(&self, state: &mut H) { - self.attesting_indices.hash(state); - self.data.hash(state); - self.signature.as_ssz_bytes().hash(state); + match self { + IndexedAttestation::Base(att) => att.attesting_indices.hash(state), + IndexedAttestation::Electra(att) => att.attesting_indices.hash(state), + }; + self.data().hash(state); + self.signature().as_ssz_bytes().hash(state); } } @@ -166,8 +267,8 @@ mod tests { let mut rng = XorShiftRng::from_seed([42; 16]); let mut indexed_vote = IndexedAttestation::random_for_test(&mut rng); - indexed_vote.data.source.epoch = Epoch::new(source_epoch); - indexed_vote.data.target.epoch = Epoch::new(target_epoch); + indexed_vote.data_mut().source.epoch = Epoch::new(source_epoch); + indexed_vote.data_mut().target.epoch = Epoch::new(target_epoch); indexed_vote } } diff --git a/slasher/src/array.rs b/slasher/src/array.rs index b733b07c63f..77ddceb85fe 100644 --- a/slasher/src/array.rs +++ b/slasher/src/array.rs @@ -226,12 +226,12 @@ impl TargetArrayChunk for MinTargetChunk { ) -> Result, Error> { let min_target = self.chunk - .get_target(validator_index, attestation.data.source.epoch, config)?; - if attestation.data.target.epoch > min_target { + .get_target(validator_index, attestation.data().source.epoch, config)?; + if attestation.data().target.epoch > min_target { let existing_attestation = db.get_attestation_for_validator(txn, validator_index, min_target)?; - if attestation.data.source.epoch < existing_attestation.data.source.epoch { + if attestation.data().source.epoch < existing_attestation.data().source.epoch { Ok(AttesterSlashingStatus::SurroundsExisting(Box::new( existing_attestation, ))) @@ -329,12 +329,12 @@ impl TargetArrayChunk for MaxTargetChunk { ) -> Result, Error> { let max_target = self.chunk - .get_target(validator_index, attestation.data.source.epoch, config)?; - if attestation.data.target.epoch < max_target { + .get_target(validator_index, attestation.data().source.epoch, config)?; + if attestation.data().target.epoch < max_target { let existing_attestation = db.get_attestation_for_validator(txn, validator_index, max_target)?; - if existing_attestation.data.source.epoch < attestation.data.source.epoch { + if existing_attestation.data().source.epoch < attestation.data().source.epoch { Ok(AttesterSlashingStatus::SurroundedByExisting(Box::new( existing_attestation, ))) @@ -428,7 +428,7 @@ pub fn apply_attestation_for_validator( current_epoch: Epoch, config: &Config, ) -> Result, Error> { - let mut chunk_index = config.chunk_index(attestation.data.source.epoch); + let mut chunk_index = config.chunk_index(attestation.data().source.epoch); let mut current_chunk = get_chunk_for_update( db, txn, @@ -446,7 +446,7 @@ pub fn apply_attestation_for_validator( } let Some(mut start_epoch) = - T::first_start_epoch(attestation.data.source.epoch, current_epoch, config) + T::first_start_epoch(attestation.data().source.epoch, current_epoch, config) else { return Ok(slashing_status); }; @@ -465,7 +465,7 @@ pub fn apply_attestation_for_validator( chunk_index, validator_index, start_epoch, - attestation.data.target.epoch, + attestation.data().target.epoch, current_epoch, config, )?; @@ -492,7 +492,7 @@ pub fn update( let mut chunk_attestations = BTreeMap::new(); for attestation in batch { chunk_attestations - .entry(config.chunk_index(attestation.indexed.data.source.epoch)) + .entry(config.chunk_index(attestation.indexed.data().source.epoch)) .or_insert_with(Vec::new) .push(attestation); } diff --git a/slasher/src/attestation_queue.rs b/slasher/src/attestation_queue.rs index 3d23932df9f..62a1bb09455 100644 --- a/slasher/src/attestation_queue.rs +++ b/slasher/src/attestation_queue.rs @@ -47,7 +47,8 @@ impl AttestationBatch { self.attestations.push(Arc::downgrade(&indexed_record)); let attestation_data_hash = indexed_record.record.attestation_data_hash; - for &validator_index in &indexed_record.indexed.attesting_indices { + + for &validator_index in indexed_record.indexed.attesting_indices_iter() { self.attesters .entry((validator_index, attestation_data_hash)) .and_modify(|existing_entry| { @@ -56,8 +57,8 @@ impl AttestationBatch { // smaller indexed attestation. Single-bit attestations will usually be removed // completely by this process, and aggregates will only be retained if they // are not redundant with respect to a larger aggregate seen in the same batch. - if existing_entry.indexed.attesting_indices.len() - < indexed_record.indexed.attesting_indices.len() + if existing_entry.indexed.attesting_indices_len() + < indexed_record.indexed.attesting_indices_len() { *existing_entry = indexed_record.clone(); } diff --git a/slasher/src/attester_record.rs b/slasher/src/attester_record.rs index 050849e33f1..8de25160725 100644 --- a/slasher/src/attester_record.rs +++ b/slasher/src/attester_record.rs @@ -87,11 +87,13 @@ struct IndexedAttestationHeader { impl From> for AttesterRecord { fn from(indexed_attestation: IndexedAttestation) -> AttesterRecord { - let attestation_data_hash = indexed_attestation.data.tree_hash_root(); + let attestation_data_hash = indexed_attestation.data().tree_hash_root(); + let attesting_indices = + VariableList::new(indexed_attestation.attesting_indices_to_vec()).unwrap_or_default(); let header = IndexedAttestationHeader:: { - attesting_indices: indexed_attestation.attesting_indices, + attesting_indices, data_root: attestation_data_hash, - signature: indexed_attestation.signature, + signature: indexed_attestation.signature().clone(), }; let indexed_attestation_hash = header.tree_hash_root(); AttesterRecord { diff --git a/slasher/src/config.rs b/slasher/src/config.rs index 4fd74343e76..26ac8841539 100644 --- a/slasher/src/config.rs +++ b/slasher/src/config.rs @@ -166,8 +166,7 @@ impl Config { validator_chunk_index: usize, ) -> impl Iterator + 'a { attestation - .attesting_indices - .iter() + .attesting_indices_iter() .filter(move |v| self.validator_chunk_index(**v) == validator_chunk_index) .copied() } diff --git a/slasher/src/database.rs b/slasher/src/database.rs index 49d2b00a4cd..d80112c553d 100644 --- a/slasher/src/database.rs +++ b/slasher/src/database.rs @@ -438,7 +438,7 @@ impl SlasherDB { ) -> Result { // Look-up ID by hash. let id_key = IndexedAttestationIdKey::new( - indexed_attestation.data.target.epoch, + indexed_attestation.data().target.epoch, indexed_attestation_hash, ); @@ -500,7 +500,7 @@ impl SlasherDB { // Otherwise, load the indexed attestation, compute the root and cache it. let indexed_attestation = self.get_indexed_attestation(txn, indexed_id)?; - let attestation_data_root = indexed_attestation.data.tree_hash_root(); + let attestation_data_root = indexed_attestation.data().tree_hash_root(); cache.put(indexed_id, attestation_data_root); @@ -536,7 +536,7 @@ impl SlasherDB { indexed_attestation_id: IndexedAttestationId, ) -> Result, Error> { // See if there's an existing attestation for this attester. - let target_epoch = attestation.data.target.epoch; + let target_epoch = attestation.data().target.epoch; let prev_max_target = self.get_attester_max_target(validator_index, txn)?; diff --git a/slasher/src/slasher.rs b/slasher/src/slasher.rs index 066c8d63d98..40b49fbbc66 100644 --- a/slasher/src/slasher.rs +++ b/slasher/src/slasher.rs @@ -299,7 +299,7 @@ impl Slasher { self.log, "Found double-vote slashing"; "validator_index" => validator_index, - "epoch" => slashing.attestation_1.data.target.epoch, + "epoch" => slashing.attestation_1.data().target.epoch, ); slashings.insert(slashing); } @@ -325,8 +325,8 @@ impl Slasher { for indexed_record in batch { let attestation = &indexed_record.indexed; - let target_epoch = attestation.data.target.epoch; - let source_epoch = attestation.data.source.epoch; + let target_epoch = attestation.data().target.epoch; + let source_epoch = attestation.data().source.epoch; if source_epoch > target_epoch || source_epoch + self.config.history_length as u64 <= current_epoch diff --git a/slasher/src/test_utils.rs b/slasher/src/test_utils.rs index 0011df1ffb0..7ec50ab5eff 100644 --- a/slasher/src/test_utils.rs +++ b/slasher/src/test_utils.rs @@ -1,7 +1,8 @@ use std::collections::HashSet; use types::{ - AggregateSignature, AttestationData, AttesterSlashing, BeaconBlockHeader, Checkpoint, Epoch, - Hash256, IndexedAttestation, MainnetEthSpec, Signature, SignedBeaconBlockHeader, Slot, + indexed_attestation::IndexedAttestationBase, AggregateSignature, AttestationData, + AttesterSlashing, BeaconBlockHeader, Checkpoint, Epoch, Hash256, IndexedAttestation, + MainnetEthSpec, Signature, SignedBeaconBlockHeader, Slot, }; pub type E = MainnetEthSpec; @@ -12,7 +13,7 @@ pub fn indexed_att( target_epoch: u64, target_root: u64, ) -> IndexedAttestation { - IndexedAttestation { + IndexedAttestation::Base(IndexedAttestationBase { attesting_indices: attesting_indices.as_ref().to_vec().into(), data: AttestationData { slot: Slot::new(0), @@ -28,7 +29,7 @@ pub fn indexed_att( }, }, signature: AggregateSignature::empty(), - } + }) } pub fn att_slashing( @@ -66,7 +67,9 @@ pub fn slashed_validators_from_slashings(slashings: &HashSet "invalid slashing: {:#?}", slashing ); - hashset_intersection(&att1.attesting_indices, &att2.attesting_indices) + let attesting_indices_1 = att1.attesting_indices_to_vec(); + let attesting_indices_2 = att2.attesting_indices_to_vec(); + hashset_intersection(&attesting_indices_1, &attesting_indices_2) }) .collect() } @@ -82,10 +85,14 @@ pub fn slashed_validators_from_attestations( continue; } + // TODO(eip7549) in a nested loop, copying to vecs feels bad + let attesting_indices_1 = att1.attesting_indices_to_vec(); + let attesting_indices_2 = att2.attesting_indices_to_vec(); + if att1.is_double_vote(att2) || att1.is_surround_vote(att2) { slashed_validators.extend(hashset_intersection( - &att1.attesting_indices, - &att2.attesting_indices, + &attesting_indices_1, + &attesting_indices_2, )); } } diff --git a/testing/web3signer_tests/src/lib.rs b/testing/web3signer_tests/src/lib.rs index 50340d7983c..cf5d5f738d0 100644 --- a/testing/web3signer_tests/src/lib.rs +++ b/testing/web3signer_tests/src/lib.rs @@ -39,7 +39,7 @@ mod tests { use tempfile::{tempdir, TempDir}; use tokio::sync::OnceCell; use tokio::time::sleep; - use types::{*, attestation::AttestationBase}; + use types::{attestation::AttestationBase, *}; use url::Url; use validator_client::{ initialized_validators::{ diff --git a/validator_client/src/http_api/tests/keystores.rs b/validator_client/src/http_api/tests/keystores.rs index ae6b5f73d78..b6923d1c788 100644 --- a/validator_client/src/http_api/tests/keystores.rs +++ b/validator_client/src/http_api/tests/keystores.rs @@ -13,7 +13,7 @@ use rand::{rngs::SmallRng, Rng, SeedableRng}; use slashing_protection::interchange::{Interchange, InterchangeMetadata}; use std::{collections::HashMap, path::Path}; use tokio::runtime::Handle; -use types::{Address, attestation::AttestationBase}; +use types::{attestation::AttestationBase, Address}; fn new_keystore(password: ZeroizeString) -> Keystore { let keypair = Keypair::random(); From fb26e15db1a5969b7d2e2bcd7b2321645728ae84 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Thu, 25 Apr 2024 17:44:00 +0300 Subject: [PATCH 8/9] indexed attestation superstruct --- beacon_node/operation_pool/src/attestation.rs | 10 ++- .../operation_pool/src/attestation_storage.rs | 73 +++++++++++++++---- beacon_node/operation_pool/src/persistence.rs | 2 +- 3 files changed, 67 insertions(+), 18 deletions(-) diff --git a/beacon_node/operation_pool/src/attestation.rs b/beacon_node/operation_pool/src/attestation.rs index 5c6f684e722..91c628e08d8 100644 --- a/beacon_node/operation_pool/src/attestation.rs +++ b/beacon_node/operation_pool/src/attestation.rs @@ -1,4 +1,4 @@ -use crate::attestation_storage::AttestationRef; +use crate::attestation_storage::{AttestationRef, CompactIndexedAttestation}; use crate::max_cover::MaxCover; use crate::reward_cache::RewardCache; use state_processing::common::{ @@ -83,7 +83,7 @@ impl<'a, E: EthSpec> AttMaxCover<'a, E> { let fresh_validators_rewards = att .indexed - .attesting_indices + .attesting_indices() .iter() .filter_map(|&index| { if reward_cache @@ -175,7 +175,11 @@ pub fn earliest_attestation_validators( base_state: &BeaconStateBase, ) -> BitList { // Bitfield of validators whose attestations are new/fresh. - let mut new_validators = attestation.indexed.aggregation_bits.clone(); + let mut new_validators = match attestation.indexed { + CompactIndexedAttestation::Base(indexed_att) => indexed_att.aggregation_bits.clone(), + // TODO(eip7549) per the comments above, this code path is obsolete post altair fork, so maybe we should just return an empty bitlist here? + CompactIndexedAttestation::Electra(_) => todo!(), + }; let state_attestations = if attestation.checkpoint.target_epoch == state.current_epoch() { &base_state.current_epoch_attestations diff --git a/beacon_node/operation_pool/src/attestation_storage.rs b/beacon_node/operation_pool/src/attestation_storage.rs index 5641324aa91..e9b9f01af77 100644 --- a/beacon_node/operation_pool/src/attestation_storage.rs +++ b/beacon_node/operation_pool/src/attestation_storage.rs @@ -2,8 +2,8 @@ use crate::AttestationStats; use itertools::Itertools; use std::collections::HashMap; use types::{ - attestation::AttestationBase, AggregateSignature, Attestation, AttestationData, BeaconState, - BitList, Checkpoint, Epoch, EthSpec, Hash256, Slot, + attestation::{AttestationBase, AttestationElectra}, superstruct, AggregateSignature, Attestation, AttestationData, + BeaconState, BitList, BitVector, Checkpoint, Epoch, EthSpec, Hash256, Slot, }; #[derive(Debug, PartialEq, Eq, Hash, Clone, Copy)] @@ -20,11 +20,18 @@ pub struct CompactAttestationData { pub target_root: Hash256, } +#[superstruct(variants(Base, Electra), variant_attributes(derive(Debug, PartialEq,)))] #[derive(Debug, PartialEq)] pub struct CompactIndexedAttestation { pub attesting_indices: Vec, + #[superstruct(only(Base), partial_getter(rename = "aggregation_bits_base"))] pub aggregation_bits: BitList, + #[superstruct(only(Electra), partial_getter(rename = "aggregation_bits_electra"))] + pub aggregation_bits: BitList, pub signature: AggregateSignature, + pub index: u64, + #[superstruct(only(Electra))] + pub committee_bits: BitVector, } #[derive(Debug)] @@ -64,17 +71,19 @@ impl SplitAttestation { target_root: attestation.data().target.root, }; - let aggregation_bits = match attestation.clone() { - Attestation::Base(attn) => attn.aggregation_bits, + let indexed = match attestation.clone() { + Attestation::Base(attn) => { + CompactIndexedAttestation::Base(CompactIndexedAttestationBase { + attesting_indices, + aggregation_bits: attn.aggregation_bits, + signature: attestation.signature().clone(), + index: data.index, + }) + } // TODO(eip7549) implement electra variant Attestation::Electra(_) => todo!(), }; - let indexed = CompactIndexedAttestation { - attesting_indices, - aggregation_bits, - signature: attestation.signature().clone(), - }; Self { checkpoint, data, @@ -106,11 +115,19 @@ impl<'a, E: EthSpec> AttestationRef<'a, E> { } pub fn clone_as_attestation(&self) -> Attestation { - Attestation::Base(AttestationBase { - aggregation_bits: self.indexed.aggregation_bits.clone(), - data: self.attestation_data(), - signature: self.indexed.signature.clone(), - }) + match self.indexed { + CompactIndexedAttestation::Base(indexed_att) => Attestation::Base(AttestationBase { + aggregation_bits: indexed_att.aggregation_bits.clone(), + data: self.attestation_data(), + signature: indexed_att.signature.clone(), + }), + CompactIndexedAttestation::Electra(indexed_att) => Attestation::Electra(AttestationElectra { + aggregation_bits: indexed_att.aggregation_bits.clone(), + data: self.attestation_data(), + signature: indexed_att.signature.clone(), + committee_bits: indexed_att.committee_bits.clone(), + }), + } } } @@ -132,6 +149,34 @@ impl CheckpointKey { } impl CompactIndexedAttestation { + pub fn signers_disjoint_from(&self, other: &Self) -> bool { + match (self, other) { + (CompactIndexedAttestation::Base(this), CompactIndexedAttestation::Base(other)) => { + this.signers_disjoint_from(other) + } + (CompactIndexedAttestation::Electra(_), CompactIndexedAttestation::Electra(_)) => { + todo!() + } + // TODO(eip7549) is a mix of electra and base compact indexed attestations an edge case we need to deal with? + _ => false, + } + } + + pub fn aggregate(&mut self, other: &Self) { + match (self, other) { + (CompactIndexedAttestation::Base(this), CompactIndexedAttestation::Base(other)) => { + this.aggregate(other) + } + (CompactIndexedAttestation::Electra(_), CompactIndexedAttestation::Electra(_)) => { + todo!() + } + // TODO(eip7549) is a mix of electra and base compact indexed attestations an edge case we need to deal with? + _ => (), + } + } +} + +impl CompactIndexedAttestationBase { pub fn signers_disjoint_from(&self, other: &Self) -> bool { self.aggregation_bits .intersection(&other.aggregation_bits) diff --git a/beacon_node/operation_pool/src/persistence.rs b/beacon_node/operation_pool/src/persistence.rs index ef749a220db..8e4e5e2898d 100644 --- a/beacon_node/operation_pool/src/persistence.rs +++ b/beacon_node/operation_pool/src/persistence.rs @@ -76,7 +76,7 @@ impl PersistedOperationPool { .map(|att| { ( att.clone_as_attestation(), - att.indexed.attesting_indices.clone(), + att.indexed.attesting_indices().clone(), ) }) .collect(); From af73ddee474d08b546f21470955f790143e8ddcb Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Tue, 30 Apr 2024 13:19:22 +0300 Subject: [PATCH 9/9] updated TODOs --- beacon_node/beacon_chain/src/beacon_chain.rs | 2 +- beacon_node/beacon_chain/src/early_attester_cache.rs | 1 + beacon_node/beacon_chain/src/observed_aggregates.rs | 6 +++--- beacon_node/beacon_chain/src/test_utils.rs | 3 ++- beacon_node/http_api/src/block_packing_efficiency.rs | 2 +- beacon_node/operation_pool/src/attestation.rs | 2 +- beacon_node/operation_pool/src/attestation_storage.rs | 6 +++--- .../state_processing/src/common/get_attesting_indices.rs | 2 +- .../state_processing/src/common/get_indexed_attestation.rs | 2 +- .../src/per_block_processing/process_operations.rs | 2 +- slasher/src/test_utils.rs | 3 ++- 11 files changed, 17 insertions(+), 14 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 698159b5351..d8a546257be 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -1917,7 +1917,7 @@ impl BeaconChain { }; drop(cache_timer); - // TODO(eip7549) implement electra variant + // TODO(electra) implement electra variant Ok(Attestation::Base(AttestationBase { aggregation_bits: BitList::with_capacity(committee_len)?, data: AttestationData { diff --git a/beacon_node/beacon_chain/src/early_attester_cache.rs b/beacon_node/beacon_chain/src/early_attester_cache.rs index d7135d94aa2..595f941235c 100644 --- a/beacon_node/beacon_chain/src/early_attester_cache.rs +++ b/beacon_node/beacon_chain/src/early_attester_cache.rs @@ -123,6 +123,7 @@ impl EarlyAttesterCache { item.committee_lengths .get_committee_length::(request_slot, request_index, spec)?; + // TODO(electra) make fork-agnostic let attestation = Attestation::Base(AttestationBase { aggregation_bits: BitList::with_capacity(committee_len) .map_err(BeaconStateError::from)?, diff --git a/beacon_node/beacon_chain/src/observed_aggregates.rs b/beacon_node/beacon_chain/src/observed_aggregates.rs index d1e9b12e272..a83eb620788 100644 --- a/beacon_node/beacon_chain/src/observed_aggregates.rs +++ b/beacon_node/beacon_chain/src/observed_aggregates.rs @@ -107,7 +107,7 @@ impl SubsetItem for Attestation { fn is_subset(&self, other: &Self::Item) -> bool { match self { Attestation::Base(att) => att.aggregation_bits.is_subset(other), - // TODO(eip7549) implement electra variant + // TODO(electra) implement electra variant Attestation::Electra(_) => todo!(), } } @@ -115,7 +115,7 @@ impl SubsetItem for Attestation { fn is_superset(&self, other: &Self::Item) -> bool { match self { Attestation::Base(att) => other.is_subset(&att.aggregation_bits), - // TODO(eip7549) implement electra variant + // TODO(electra) implement electra variant Attestation::Electra(_) => todo!(), } } @@ -124,7 +124,7 @@ impl SubsetItem for Attestation { fn get_item(&self) -> Self::Item { match self { Attestation::Base(att) => att.aggregation_bits.clone(), - // TODO(eip7549) implement electra variant + // TODO(electra) implement electra variant Attestation::Electra(_) => todo!(), } } diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index 190c3eac74e..4a53f64fcac 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -1032,6 +1032,7 @@ where *state.get_block_root(target_slot)? }; + // TODO(electra) make test fork-agnostic Ok(Attestation::Base(AttestationBase { aggregation_bits: BitList::with_capacity(committee_len)?, data: AttestationData { @@ -1515,7 +1516,7 @@ where attestation_2.data_mut().target.epoch = target2.unwrap_or(fork.epoch); for attestation in &mut [&mut attestation_1, &mut attestation_2] { - // TODO(eip7549) we could explore iter mut here + // TODO(electra) we could explore iter mut here for i in attestation.attesting_indices_to_vec() { let sk = &self.validator_keypairs[i as usize].sk; diff --git a/beacon_node/http_api/src/block_packing_efficiency.rs b/beacon_node/http_api/src/block_packing_efficiency.rs index b990ee8297d..3e511c25dff 100644 --- a/beacon_node/http_api/src/block_packing_efficiency.rs +++ b/beacon_node/http_api/src/block_packing_efficiency.rs @@ -132,7 +132,7 @@ impl PackingEfficiencyHandler { } } } - // TODO(eip7549) implement electra variant + // TODO(electra) implement electra variant Attestation::Electra(_) => { todo!() } diff --git a/beacon_node/operation_pool/src/attestation.rs b/beacon_node/operation_pool/src/attestation.rs index 91c628e08d8..60074cde740 100644 --- a/beacon_node/operation_pool/src/attestation.rs +++ b/beacon_node/operation_pool/src/attestation.rs @@ -177,7 +177,7 @@ pub fn earliest_attestation_validators( // Bitfield of validators whose attestations are new/fresh. let mut new_validators = match attestation.indexed { CompactIndexedAttestation::Base(indexed_att) => indexed_att.aggregation_bits.clone(), - // TODO(eip7549) per the comments above, this code path is obsolete post altair fork, so maybe we should just return an empty bitlist here? + // TODO(electra) per the comments above, this code path is obsolete post altair fork, so maybe we should just return an empty bitlist here? CompactIndexedAttestation::Electra(_) => todo!(), }; diff --git a/beacon_node/operation_pool/src/attestation_storage.rs b/beacon_node/operation_pool/src/attestation_storage.rs index e9b9f01af77..90fdf3cdb81 100644 --- a/beacon_node/operation_pool/src/attestation_storage.rs +++ b/beacon_node/operation_pool/src/attestation_storage.rs @@ -80,7 +80,7 @@ impl SplitAttestation { index: data.index, }) } - // TODO(eip7549) implement electra variant + // TODO(electra) implement electra variant Attestation::Electra(_) => todo!(), }; @@ -157,7 +157,7 @@ impl CompactIndexedAttestation { (CompactIndexedAttestation::Electra(_), CompactIndexedAttestation::Electra(_)) => { todo!() } - // TODO(eip7549) is a mix of electra and base compact indexed attestations an edge case we need to deal with? + // TODO(electra) is a mix of electra and base compact indexed attestations an edge case we need to deal with? _ => false, } } @@ -170,7 +170,7 @@ impl CompactIndexedAttestation { (CompactIndexedAttestation::Electra(_), CompactIndexedAttestation::Electra(_)) => { todo!() } - // TODO(eip7549) is a mix of electra and base compact indexed attestations an edge case we need to deal with? + // TODO(electra) is a mix of electra and base compact indexed attestations an edge case we need to deal with? _ => (), } } diff --git a/consensus/state_processing/src/common/get_attesting_indices.rs b/consensus/state_processing/src/common/get_attesting_indices.rs index 7a1f9f3f480..6b8ce6f9372 100644 --- a/consensus/state_processing/src/common/get_attesting_indices.rs +++ b/consensus/state_processing/src/common/get_attesting_indices.rs @@ -32,7 +32,7 @@ pub fn get_attesting_indices_from_state( Attestation::Base(att) => { get_attesting_indices::(committee.committee, &att.aggregation_bits) } - // TODO(eip7549) implement get_attesting_indices for electra + // TODO(electra) implement get_attesting_indices for electra Attestation::Electra(_) => todo!(), } } diff --git a/consensus/state_processing/src/common/get_indexed_attestation.rs b/consensus/state_processing/src/common/get_indexed_attestation.rs index b68d5ab61e8..d6c7512961c 100644 --- a/consensus/state_processing/src/common/get_indexed_attestation.rs +++ b/consensus/state_processing/src/common/get_indexed_attestation.rs @@ -13,7 +13,7 @@ pub fn get_indexed_attestation( ) -> Result> { let attesting_indices = match attestation { Attestation::Base(att) => get_attesting_indices::(committee, &att.aggregation_bits)?, - // TODO(eip7549) implement get_attesting_indices for electra + // TODO(electra) implement get_attesting_indices for electra Attestation::Electra(_) => todo!(), }; diff --git a/consensus/state_processing/src/per_block_processing/process_operations.rs b/consensus/state_processing/src/per_block_processing/process_operations.rs index 48b2676c157..cb677f5514f 100644 --- a/consensus/state_processing/src/per_block_processing/process_operations.rs +++ b/consensus/state_processing/src/per_block_processing/process_operations.rs @@ -95,7 +95,7 @@ pub mod base { } } Attestation::Electra(_) => { - // TODO(eip7549) pending attestations are only phase 0 + // TODO(electra) pending attestations are only phase 0 // so we should just raise a relevant error here todo!() } diff --git a/slasher/src/test_utils.rs b/slasher/src/test_utils.rs index 7ec50ab5eff..c46b2f39492 100644 --- a/slasher/src/test_utils.rs +++ b/slasher/src/test_utils.rs @@ -13,6 +13,7 @@ pub fn indexed_att( target_epoch: u64, target_root: u64, ) -> IndexedAttestation { + // TODO(electra) make fork-agnostic IndexedAttestation::Base(IndexedAttestationBase { attesting_indices: attesting_indices.as_ref().to_vec().into(), data: AttestationData { @@ -85,7 +86,7 @@ pub fn slashed_validators_from_attestations( continue; } - // TODO(eip7549) in a nested loop, copying to vecs feels bad + // TODO(electra) in a nested loop, copying to vecs feels bad let attesting_indices_1 = att1.attesting_indices_to_vec(); let attesting_indices_2 = att2.attesting_indices_to_vec();