Add DataColumnSidecar gossip topic and message handling (#6147)

* Add `DataColumnSidecar` gossip topic and verification (#5050 and #5783).

* Remove gossip verification changes (#5783).

* Merge branch 'unstable' into data-column-gossip

# Conflicts:
#	beacon_node/beacon_chain/src/data_column_verification.rs
#	beacon_node/beacon_chain/src/lib.rs

* Add gossip cache timeout for data columns. Rename data column metrics for consistency.

* Remove usage of `unimplemented!` and address review comments.

* Remove unnused `GossipDataColumnError` variants and address review comments.

* Merge branch 'unstable' into data-column-gossip

* Update Cargo.lock

* Arc `ChainSpec` in discovery to avoid performance regression when needing to clone it repeatedly.
This commit is contained in:
Jimmy Chen
2024-07-25 16:05:18 +10:00
committed by GitHub
parent a2ab26c327
commit 4e5a363a4f
26 changed files with 907 additions and 31 deletions

View File

@@ -43,7 +43,7 @@ use std::{
time::{Duration, Instant},
};
use tokio::sync::mpsc;
use types::{EnrForkId, EthSpec};
use types::{ChainSpec, EnrForkId, EthSpec};
mod subnet_predicate;
pub use subnet_predicate::subnet_predicate;
@@ -192,6 +192,7 @@ pub struct Discovery<E: EthSpec> {
/// Logger for the discovery behaviour.
log: slog::Logger,
spec: Arc<ChainSpec>,
}
impl<E: EthSpec> Discovery<E> {
@@ -201,6 +202,7 @@ impl<E: EthSpec> Discovery<E> {
config: &NetworkConfig,
network_globals: Arc<NetworkGlobals<E>>,
log: &slog::Logger,
spec: &ChainSpec,
) -> error::Result<Self> {
let log = log.clone();
@@ -325,6 +327,7 @@ impl<E: EthSpec> Discovery<E> {
update_ports,
log,
enr_dir,
spec: Arc::new(spec.clone()),
})
}
@@ -548,6 +551,8 @@ impl<E: EthSpec> Discovery<E> {
)
.map_err(|e| format!("{:?}", e))?;
}
// Data column subnets are computed from node ID. No subnet bitfield in the ENR.
Subnet::DataColumn(_) => return Ok(()),
}
// replace the global version
@@ -753,7 +758,8 @@ impl<E: EthSpec> Discovery<E> {
// Only start a discovery query if we have a subnet to look for.
if !filtered_subnet_queries.is_empty() {
// build the subnet predicate as a combination of the eth2_fork_predicate and the subnet predicate
let subnet_predicate = subnet_predicate::<E>(filtered_subnets, &self.log);
let subnet_predicate =
subnet_predicate::<E>(filtered_subnets, &self.log, self.spec.clone());
debug!(
self.log,
@@ -867,6 +873,7 @@ impl<E: EthSpec> Discovery<E> {
let query_str = match query.subnet {
Subnet::Attestation(_) => "attestation",
Subnet::SyncCommittee(_) => "sync_committee",
Subnet::DataColumn(_) => "data_column",
};
if let Some(v) = metrics::get_int_counter(
@@ -879,8 +886,11 @@ impl<E: EthSpec> Discovery<E> {
self.add_subnet_query(query.subnet, query.min_ttl, query.retries + 1);
// Check the specific subnet against the enr
let subnet_predicate =
subnet_predicate::<E>(vec![query.subnet], &self.log);
let subnet_predicate = subnet_predicate::<E>(
vec![query.subnet],
&self.log,
self.spec.clone(),
);
r.clone()
.into_iter()
@@ -1194,6 +1204,7 @@ mod tests {
}
async fn build_discovery() -> Discovery<E> {
let spec = ChainSpec::default();
let keypair = secp256k1::Keypair::generate();
let mut config = NetworkConfig::default();
config.set_listening_addr(crate::ListenAddress::unused_v4_ports());
@@ -1212,7 +1223,7 @@ mod tests {
&log,
);
let keypair = keypair.into();
Discovery::new(keypair, &config, Arc::new(globals), &log)
Discovery::new(keypair, &config, Arc::new(globals), &log, &spec)
.await
.unwrap()
}

View File

@@ -1,11 +1,17 @@
//! The subnet predicate used for searching for a particular subnet.
use super::*;
use crate::types::{EnrAttestationBitfield, EnrSyncCommitteeBitfield};
use itertools::Itertools;
use slog::trace;
use std::ops::Deref;
use types::{ChainSpec, DataColumnSubnetId};
/// Returns the predicate for a given subnet.
pub fn subnet_predicate<E>(subnets: Vec<Subnet>, log: &slog::Logger) -> impl Fn(&Enr) -> bool + Send
pub fn subnet_predicate<E>(
subnets: Vec<Subnet>,
log: &slog::Logger,
spec: Arc<ChainSpec>,
) -> impl Fn(&Enr) -> bool + Send
where
E: EthSpec,
{
@@ -19,10 +25,13 @@ where
};
// Pre-fork/fork-boundary enrs may not contain a syncnets field.
// Don't return early here
// Don't return early here.
let sync_committee_bitfield: Result<EnrSyncCommitteeBitfield<E>, _> =
enr.sync_committee_bitfield::<E>();
// TODO(das): compute from enr
let custody_subnet_count = spec.custody_requirement;
let predicate = subnets.iter().any(|subnet| match subnet {
Subnet::Attestation(s) => attestation_bitfield
.get(*s.deref() as usize)
@@ -30,6 +39,14 @@ where
Subnet::SyncCommittee(s) => sync_committee_bitfield
.as_ref()
.map_or(false, |b| b.get(*s.deref() as usize).unwrap_or(false)),
Subnet::DataColumn(s) => {
let mut subnets = DataColumnSubnetId::compute_custody_subnets::<E>(
enr.node_id().raw().into(),
custody_subnet_count,
&spec,
);
subnets.contains(s)
}
});
if !predicate {

View File

@@ -1027,6 +1027,10 @@ impl<E: EthSpec> PeerManager<E> {
.or_default()
.insert(id);
}
// TODO(das) to be implemented. We're not pruning data column peers yet
// because data column topics are subscribed as core topics until we
// implement recomputing data column subnets.
Subnet::DataColumn(_) => {}
}
}
}

View File

@@ -94,6 +94,15 @@ impl<E: EthSpec> PeerInfo<E> {
.syncnets()
.map_or(false, |s| s.get(**id as usize).unwrap_or(false))
}
Subnet::DataColumn(_) => {
// TODO(das): Pending spec PR https://github.com/ethereum/consensus-specs/pull/3821
// We should use MetaDataV3 for peer selection rather than
// looking at subscribed peers (current behavior). Until MetaDataV3 is
// implemented, this is the perhaps the only viable option on the current devnet
// as the peer count is low and it's important to identify supernodes to get a
// good distribution of peers across subnets.
return true;
}
}
}
false

View File

@@ -22,6 +22,8 @@ pub struct GossipCache {
beacon_block: Option<Duration>,
/// Timeout for blobs.
blob_sidecar: Option<Duration>,
/// Timeout for data columns.
data_column_sidecar: Option<Duration>,
/// Timeout for aggregate attestations.
aggregates: Option<Duration>,
/// Timeout for attestations.
@@ -51,6 +53,8 @@ pub struct GossipCacheBuilder {
beacon_block: Option<Duration>,
/// Timeout for blob sidecars.
blob_sidecar: Option<Duration>,
/// Timeout for data column sidecars.
data_column_sidecar: Option<Duration>,
/// Timeout for aggregate attestations.
aggregates: Option<Duration>,
/// Timeout for attestations.
@@ -152,6 +156,7 @@ impl GossipCacheBuilder {
default_timeout,
beacon_block,
blob_sidecar,
data_column_sidecar,
aggregates,
attestation,
voluntary_exit,
@@ -168,6 +173,7 @@ impl GossipCacheBuilder {
topic_msgs: HashMap::default(),
beacon_block: beacon_block.or(default_timeout),
blob_sidecar: blob_sidecar.or(default_timeout),
data_column_sidecar: data_column_sidecar.or(default_timeout),
aggregates: aggregates.or(default_timeout),
attestation: attestation.or(default_timeout),
voluntary_exit: voluntary_exit.or(default_timeout),
@@ -194,6 +200,7 @@ impl GossipCache {
let expire_timeout = match topic.kind() {
GossipKind::BeaconBlock => self.beacon_block,
GossipKind::BlobSidecar(_) => self.blob_sidecar,
GossipKind::DataColumnSidecar(_) => self.data_column_sidecar,
GossipKind::BeaconAggregateAndProof => self.aggregates,
GossipKind::Attestation(_) => self.attestation,
GossipKind::VoluntaryExit => self.voluntary_exit,

View File

@@ -39,10 +39,10 @@ use std::{
sync::Arc,
task::{Context, Poll},
};
use types::ForkName;
use types::{
consts::altair::SYNC_COMMITTEE_SUBNET_COUNT, EnrForkId, EthSpec, ForkContext, Slot, SubnetId,
};
use types::{ChainSpec, ForkName};
use utils::{build_transport, strip_peer_id, Context as ServiceContext, MAX_CONNECTIONS_PER_PEER};
pub mod api_types;
@@ -327,6 +327,7 @@ impl<E: EthSpec> Network<E> {
&config,
network_globals.clone(),
&log,
ctx.chain_spec,
)
.await?;
// start searching for peers
@@ -1018,6 +1019,7 @@ impl<E: EthSpec> Network<E> {
return;
}
let spec = Arc::new(self.fork_context.spec.clone());
let filtered: Vec<SubnetDiscovery> = subnets_to_discover
.into_iter()
.filter(|s| {
@@ -1053,7 +1055,7 @@ impl<E: EthSpec> Network<E> {
// If we connect to the cached peers before the discovery query starts, then we potentially
// save a costly discovery query.
} else {
self.dial_cached_enrs_in_subnet(s.subnet);
self.dial_cached_enrs_in_subnet(s.subnet, spec.clone());
true
}
})
@@ -1217,8 +1219,8 @@ impl<E: EthSpec> Network<E> {
/// Dial cached Enrs in discovery service that are in the given `subnet_id` and aren't
/// in Connected, Dialing or Banned state.
fn dial_cached_enrs_in_subnet(&mut self, subnet: Subnet) {
let predicate = subnet_predicate::<E>(vec![subnet], &self.log);
fn dial_cached_enrs_in_subnet(&mut self, subnet: Subnet, spec: Arc<ChainSpec>) {
let predicate = subnet_predicate::<E>(vec![subnet], &self.log, spec);
let peers_to_dial: Vec<Enr> = self
.discovery()
.cached_enrs()

View File

@@ -8,13 +8,13 @@ use std::io::{Error, ErrorKind};
use std::sync::Arc;
use types::{
Attestation, AttestationBase, AttestationElectra, AttesterSlashing, AttesterSlashingBase,
AttesterSlashingElectra, BlobSidecar, EthSpec, ForkContext, ForkName,
LightClientFinalityUpdate, LightClientOptimisticUpdate, ProposerSlashing,
SignedAggregateAndProof, SignedAggregateAndProofBase, SignedAggregateAndProofElectra,
SignedBeaconBlock, SignedBeaconBlockAltair, SignedBeaconBlockBase, SignedBeaconBlockBellatrix,
SignedBeaconBlockCapella, SignedBeaconBlockDeneb, SignedBeaconBlockElectra,
SignedBlsToExecutionChange, SignedContributionAndProof, SignedVoluntaryExit, SubnetId,
SyncCommitteeMessage, SyncSubnetId,
AttesterSlashingElectra, BlobSidecar, DataColumnSidecar, DataColumnSubnetId, EthSpec,
ForkContext, ForkName, LightClientFinalityUpdate, LightClientOptimisticUpdate,
ProposerSlashing, SignedAggregateAndProof, SignedAggregateAndProofBase,
SignedAggregateAndProofElectra, SignedBeaconBlock, SignedBeaconBlockAltair,
SignedBeaconBlockBase, SignedBeaconBlockBellatrix, SignedBeaconBlockCapella,
SignedBeaconBlockDeneb, SignedBeaconBlockElectra, SignedBlsToExecutionChange,
SignedContributionAndProof, SignedVoluntaryExit, SubnetId, SyncCommitteeMessage, SyncSubnetId,
};
#[derive(Debug, Clone, PartialEq)]
@@ -23,6 +23,8 @@ pub enum PubsubMessage<E: EthSpec> {
BeaconBlock(Arc<SignedBeaconBlock<E>>),
/// Gossipsub message providing notification of a [`BlobSidecar`] along with the subnet id where it was received.
BlobSidecar(Box<(u64, Arc<BlobSidecar<E>>)>),
/// Gossipsub message providing notification of a [`DataColumnSidecar`] along with the subnet id where it was received.
DataColumnSidecar(Box<(DataColumnSubnetId, Arc<DataColumnSidecar<E>>)>),
/// Gossipsub message providing notification of a Aggregate attestation and associated proof.
AggregateAndProofAttestation(Box<SignedAggregateAndProof<E>>),
/// Gossipsub message providing notification of a raw un-aggregated attestation with its shard id.
@@ -119,6 +121,9 @@ impl<E: EthSpec> PubsubMessage<E> {
PubsubMessage::BlobSidecar(blob_sidecar_data) => {
GossipKind::BlobSidecar(blob_sidecar_data.0)
}
PubsubMessage::DataColumnSidecar(column_sidecar_data) => {
GossipKind::DataColumnSidecar(column_sidecar_data.0)
}
PubsubMessage::AggregateAndProofAttestation(_) => GossipKind::BeaconAggregateAndProof,
PubsubMessage::Attestation(attestation_data) => {
GossipKind::Attestation(attestation_data.0)
@@ -270,6 +275,36 @@ impl<E: EthSpec> PubsubMessage<E> {
)),
}
}
GossipKind::DataColumnSidecar(subnet_id) => {
match fork_context.from_context_bytes(gossip_topic.fork_digest) {
// TODO(das): Remove Deneb fork
Some(fork) if fork.deneb_enabled() => {
let col_sidecar = Arc::new(
DataColumnSidecar::from_ssz_bytes(data)
.map_err(|e| format!("{:?}", e))?,
);
let peer_das_enabled =
fork_context.spec.is_peer_das_enabled_for_epoch(
col_sidecar.slot().epoch(E::slots_per_epoch()),
);
if peer_das_enabled {
Ok(PubsubMessage::DataColumnSidecar(Box::new((
*subnet_id,
col_sidecar,
))))
} else {
Err(format!(
"data_column_sidecar topic invalid for given fork digest {:?}",
gossip_topic.fork_digest
))
}
}
Some(_) | None => Err(format!(
"data_column_sidecar topic invalid for given fork digest {:?}",
gossip_topic.fork_digest
)),
}
}
GossipKind::VoluntaryExit => {
let voluntary_exit = SignedVoluntaryExit::from_ssz_bytes(data)
.map_err(|e| format!("{:?}", e))?;
@@ -373,6 +408,7 @@ impl<E: EthSpec> PubsubMessage<E> {
match &self {
PubsubMessage::BeaconBlock(data) => data.as_ssz_bytes(),
PubsubMessage::BlobSidecar(data) => data.1.as_ssz_bytes(),
PubsubMessage::DataColumnSidecar(data) => data.1.as_ssz_bytes(),
PubsubMessage::AggregateAndProofAttestation(data) => data.as_ssz_bytes(),
PubsubMessage::VoluntaryExit(data) => data.as_ssz_bytes(),
PubsubMessage::ProposerSlashing(data) => data.as_ssz_bytes(),
@@ -402,6 +438,12 @@ impl<E: EthSpec> std::fmt::Display for PubsubMessage<E> {
data.1.slot(),
data.1.index,
),
PubsubMessage::DataColumnSidecar(data) => write!(
f,
"DataColumnSidecar: slot: {}, column index: {}",
data.1.slot(),
data.1.index,
),
PubsubMessage::AggregateAndProofAttestation(att) => write!(
f,
"Aggregate and Proof: slot: {}, index: {:?}, aggregator_index: {}",

View File

@@ -1,6 +1,6 @@
use serde::Serialize;
use std::time::Instant;
use types::{SubnetId, SyncSubnetId};
use types::{DataColumnSubnetId, SubnetId, SyncSubnetId};
/// Represents a subnet on an attestation or sync committee `SubnetId`.
///
@@ -12,6 +12,8 @@ pub enum Subnet {
Attestation(SubnetId),
/// Represents a gossipsub sync committee subnet and the metadata `syncnets` field.
SyncCommittee(SyncSubnetId),
/// Represents a gossipsub data column subnet.
DataColumn(DataColumnSubnetId),
}
/// A subnet to discover peers on along with the instant after which it's no longer useful.

View File

@@ -1,7 +1,7 @@
use gossipsub::{IdentTopic as Topic, TopicHash};
use serde::{Deserialize, Serialize};
use strum::AsRefStr;
use types::{ChainSpec, EthSpec, ForkName, SubnetId, SyncSubnetId, Unsigned};
use types::{ChainSpec, DataColumnSubnetId, EthSpec, ForkName, SubnetId, SyncSubnetId, Unsigned};
use crate::Subnet;
@@ -14,6 +14,7 @@ pub const BEACON_BLOCK_TOPIC: &str = "beacon_block";
pub const BEACON_AGGREGATE_AND_PROOF_TOPIC: &str = "beacon_aggregate_and_proof";
pub const BEACON_ATTESTATION_PREFIX: &str = "beacon_attestation_";
pub const BLOB_SIDECAR_PREFIX: &str = "blob_sidecar_";
pub const DATA_COLUMN_SIDECAR_PREFIX: &str = "data_column_sidecar_";
pub const VOLUNTARY_EXIT_TOPIC: &str = "voluntary_exit";
pub const PROPOSER_SLASHING_TOPIC: &str = "proposer_slashing";
pub const ATTESTER_SLASHING_TOPIC: &str = "attester_slashing";
@@ -112,6 +113,8 @@ pub enum GossipKind {
BeaconAggregateAndProof,
/// Topic for publishing BlobSidecars.
BlobSidecar(u64),
/// Topic for publishing DataColumnSidecars.
DataColumnSidecar(DataColumnSubnetId),
/// Topic for publishing raw attestations on a particular subnet.
#[strum(serialize = "beacon_attestation")]
Attestation(SubnetId),
@@ -144,6 +147,9 @@ impl std::fmt::Display for GossipKind {
GossipKind::BlobSidecar(blob_index) => {
write!(f, "{}{}", BLOB_SIDECAR_PREFIX, blob_index)
}
GossipKind::DataColumnSidecar(column_index) => {
write!(f, "{}{}", DATA_COLUMN_SIDECAR_PREFIX, **column_index)
}
x => f.write_str(x.as_ref()),
}
}
@@ -231,6 +237,7 @@ impl GossipTopic {
match self.kind() {
GossipKind::Attestation(subnet_id) => Some(Subnet::Attestation(*subnet_id)),
GossipKind::SyncCommitteeMessage(subnet_id) => Some(Subnet::SyncCommittee(*subnet_id)),
GossipKind::DataColumnSidecar(subnet_id) => Some(Subnet::DataColumn(*subnet_id)),
_ => None,
}
}
@@ -269,6 +276,9 @@ impl std::fmt::Display for GossipTopic {
GossipKind::BlobSidecar(blob_index) => {
format!("{}{}", BLOB_SIDECAR_PREFIX, blob_index)
}
GossipKind::DataColumnSidecar(index) => {
format!("{}{}", DATA_COLUMN_SIDECAR_PREFIX, *index)
}
GossipKind::BlsToExecutionChange => BLS_TO_EXECUTION_CHANGE_TOPIC.into(),
GossipKind::LightClientFinalityUpdate => LIGHT_CLIENT_FINALITY_UPDATE.into(),
GossipKind::LightClientOptimisticUpdate => LIGHT_CLIENT_OPTIMISTIC_UPDATE.into(),
@@ -289,6 +299,7 @@ impl From<Subnet> for GossipKind {
match subnet_id {
Subnet::Attestation(s) => GossipKind::Attestation(s),
Subnet::SyncCommittee(s) => GossipKind::SyncCommitteeMessage(s),
Subnet::DataColumn(s) => GossipKind::DataColumnSidecar(s),
}
}
}
@@ -312,6 +323,10 @@ fn subnet_topic_index(topic: &str) -> Option<GossipKind> {
)));
} else if let Some(index) = topic.strip_prefix(BLOB_SIDECAR_PREFIX) {
return Some(GossipKind::BlobSidecar(index.parse::<u64>().ok()?));
} else if let Some(index) = topic.strip_prefix(DATA_COLUMN_SIDECAR_PREFIX) {
return Some(GossipKind::DataColumnSidecar(DataColumnSubnetId::new(
index.parse::<u64>().ok()?,
)));
}
None
}