Skip to content

Commit

Permalink
Fix /eth/v1/beacon/blob_sidecars/{block_id} endpoint.
Browse files Browse the repository at this point in the history
  • Loading branch information
Tumas committed Dec 30, 2024
1 parent 7442a4c commit db2c98f
Show file tree
Hide file tree
Showing 13 changed files with 80 additions and 47 deletions.
2 changes: 0 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion fork_choice_control/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,8 @@ impl<P: Preset> Context<P> {
}

pub fn on_blob_sidecar(&mut self, blob_sidecar: BlobSidecar<P>) -> Option<P2pMessage<P>> {
let subnet_id = misc::compute_subnet_for_blob_sidecar(self.config(), &blob_sidecar);
let subnet_id = misc::compute_subnet_for_blob_sidecar(self.config(), &blob_sidecar)
.expect("cannot compute subnet_id for given blob_sidecar");

self.controller().on_gossip_blob_sidecar(
Arc::new(blob_sidecar),
Expand Down
1 change: 0 additions & 1 deletion fork_choice_store/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,5 @@ tap = { workspace = true }
thiserror = { workspace = true }
transition_functions = { workspace = true }
tynm = { workspace = true }
typenum = { workspace = true }
types = { workspace = true }
unwrap_none = { workspace = true }
15 changes: 7 additions & 8 deletions fork_choice_store/src/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ use transition_functions::{
combined,
unphased::{self, ProcessSlots, StateRootPolicy},
};
use typenum::Unsigned as _;
use types::{
combined::{
Attestation, AttesterSlashing, AttestingIndices, BeaconState, SignedAggregateAndProof,
Expand Down Expand Up @@ -1699,12 +1698,11 @@ impl<P: Preset> Store<P> {
let block_header = blob_sidecar.signed_block_header.message;

// [REJECT] The sidecar's index is consistent with MAX_BLOBS_PER_BLOCK -- i.e. blob_sidecar.index < MAX_BLOBS_PER_BLOCK.
let max_blobs_per_block =
if self.chain_config().phase_at_slot::<P>(block_header.slot) >= Phase::Electra {
P::MaxBlobsPerBlockElectra::U64
} else {
P::MaxBlobsPerBlock::U64
};
let max_blobs_per_block = self
.chain_config()
.phase_at_slot::<P>(block_header.slot)
.max_blobs_per_block::<P>()
.unwrap_or_default();

ensure!(
blob_sidecar.index < max_blobs_per_block,
Expand All @@ -1713,7 +1711,8 @@ impl<P: Preset> Store<P> {

// [REJECT] The sidecar is for the correct subnet -- i.e. compute_subnet_for_blob_sidecar(blob_sidecar.index) == subnet_id.
if let Some(actual) = origin.subnet_id() {
let expected = misc::compute_subnet_for_blob_sidecar(&self.chain_config, &blob_sidecar);
let expected =
misc::compute_subnet_for_blob_sidecar(&self.chain_config, &blob_sidecar)?;

ensure!(
actual == expected,
Expand Down
2 changes: 2 additions & 0 deletions helper_functions/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ pub(crate) enum Error {
AttestationSourceMismatch,
#[error("attesting indices are not sorted and unique")]
AttestingIndicesNotSortedAndUnique,
#[error("cannot compute subnet for blob sidecar")]
BlobSidecarSubnetNotAvailable,
#[error("committee index is out of bounds")]
CommitteeIndexOutOfBounds,
#[error("aggregation bitlist length {aggregation_bitlist_length} does not match committee length {committee_length}")]
Expand Down
15 changes: 7 additions & 8 deletions helper_functions/src/misc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ use types::{
Blob, BlobCommitmentInclusionProof, BlobIndex, KzgCommitment, KzgProof, VersionedHash,
},
},
nonstandard::Phase,
phase0::{
consts::{
AttestationSubnetCount, BLS_WITHDRAWAL_PREFIX, ETH1_ADDRESS_WITHDRAWAL_PREFIX,
Expand Down Expand Up @@ -279,16 +278,16 @@ pub fn compute_subnet_for_attestation<P: Preset>(
}

/// [`compute_subnet_for_blob_sidecar`](https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.1/specs/deneb/validator.md#sidecar)
#[must_use]
pub fn compute_subnet_for_blob_sidecar<P: Preset>(
config: &Config,
blob_sidecar: &BlobSidecar<P>,
) -> SubnetId {
if config.phase_at_slot::<P>(blob_sidecar.signed_block_header.message.slot) >= Phase::Electra {
blob_sidecar.index % config.blob_sidecar_subnet_count_electra
} else {
blob_sidecar.index % config.blob_sidecar_subnet_count
}
) -> Result<SubnetId> {
let phase = config.phase_at_slot::<P>(blob_sidecar.signed_block_header.message.slot);

Ok(blob_sidecar.index
% config
.blob_sidecar_subnet_count(phase)
.ok_or(Error::BlobSidecarSubnetNotAvailable)?)
}

/// <https://github.com/ethereum/consensus-specs/blob/v1.1.0/specs/altair/validator.md#broadcast-sync-committee-message>
Expand Down
1 change: 0 additions & 1 deletion http_api/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ tokio = { workspace = true }
tokio-stream = { workspace = true }
tower-http = { workspace = true }
transition_functions = { workspace = true }
try_from_iterator = { workspace = true }
typenum = { workspace = true }
types = { workspace = true }
unwrap_none = { workspace = true }
Expand Down
39 changes: 21 additions & 18 deletions http_api/src/standard.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,10 @@ use prometheus_metrics::Metrics;
use serde::{Deserialize, Serialize};
use serde_json::Value;
use serde_with::{As, DisplayFromStr};
use ssz::{ContiguousList, SszHash as _};
use ssz::{DynamicList, SszHash as _};
use std_ext::ArcExt as _;
use tap::Pipe as _;
use tokio_stream::wrappers::{errors::BroadcastStreamRecvError, BroadcastStream};
use try_from_iterator::TryFromIterator as _;
use typenum::Unsigned as _;
use types::{
altair::{
Expand Down Expand Up @@ -941,34 +940,38 @@ pub async fn blob_sidecars<P: Preset, W: Wait>(
EthPath(block_id): EthPath<BlockId>,
EthQuery(query): EthQuery<BlobSidecarsQuery>,
headers: HeaderMap,
) -> Result<
EthResponse<ContiguousList<Arc<BlobSidecar<P>>, P::MaxBlobsPerBlock>, (), JsonOrSsz>,
Error,
> {
let block_root =
block_id::block_root(block_id, &controller, &anchor_checkpoint_provider)?.value;
) -> Result<EthResponse<DynamicList<Arc<BlobSidecar<P>>>, (), JsonOrSsz>, Error> {
let WithStatus {
value: block,
optimistic,
finalized,
} = block_id::block(block_id, &controller, &anchor_checkpoint_provider)?;

// TODO(feature/electra): support P::MaxBlobsPerBlockElectra
let version = block.phase();
let block_root = block.message().hash_tree_root();
let max_blobs_per_block = version.max_blobs_per_block::<P>().unwrap_or_default();

let blob_identifiers = query
.indices
.unwrap_or_else(|| (0..P::MaxBlobsPerBlock::U64).collect())
.unwrap_or_else(|| (0..max_blobs_per_block).collect())
.into_iter()
.map(|index| {
ensure!(
index < P::MaxBlobsPerBlock::U64,
Error::InvalidBlobIndex(index)
);

ensure!(index < max_blobs_per_block, Error::InvalidBlobIndex(index));
Ok(BlobIdentifier { block_root, index })
})
.collect::<Result<Vec<_>>>()?;

let blob_sidecars = controller.blob_sidecars_by_ids(blob_identifiers)?;
let blob_sidecars =
ContiguousList::try_from_iter(blob_sidecars.into_iter()).map_err(AnyhowError::new)?;
let blob_sidecars = DynamicList::try_from_iter_with_maximum(
blob_sidecars.into_iter(),
usize::try_from(max_blobs_per_block).map_err(AnyhowError::new)?,
)
.map_err(AnyhowError::new)?;

Ok(EthResponse::json_or_ssz(blob_sidecars, &headers)?)
Ok(EthResponse::json_or_ssz(blob_sidecars, &headers)?
.execution_optimistic(optimistic)
.finalized(finalized)
.version(version))
}

/// `POST /eth/v1/beacon/blocks`
Expand Down
19 changes: 12 additions & 7 deletions p2p/src/network.rs
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ impl<P: Preset> Network<P> {
ValidatorToP2p::PublishContributionAndProof(contribution_and_proof) => {
self.publish_contribution_and_proof(contribution_and_proof);
}
}
};
},

message = self.channels.sync_to_p2p_rx.select_next_some() => {
Expand Down Expand Up @@ -550,14 +550,19 @@ impl<P: Preset> Network<P> {
let subnet_id =
misc::compute_subnet_for_blob_sidecar(self.controller.chain_config(), &blob_sidecar);

let blob_identifier: BlobIdentifier = blob_sidecar.as_ref().into();
match subnet_id {
Ok(subnet_id) => {
let blob_identifier: BlobIdentifier = blob_sidecar.as_ref().into();

debug!("publishing blob sidecar: {blob_identifier:?}, subnet_id: {subnet_id}");
debug!("publishing blob sidecar: {blob_identifier:?}, subnet_id: {subnet_id}");

self.publish(PubsubMessage::BlobSidecar(Box::new((
subnet_id,
blob_sidecar,
))));
self.publish(PubsubMessage::BlobSidecar(Box::new((
subnet_id,
blob_sidecar,
))));
}
Err(error) => warn!("unable to publish blob sidecar: {error:?}"),
}
}

fn publish_singular_attestation(&self, attestation: Arc<Attestation<P>>, subnet_id: SubnetId) {
Expand Down
7 changes: 7 additions & 0 deletions ssz/src/dynamic_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use core::fmt::Debug;

use derivative::Derivative;
use derive_more::{AsRef, Deref};
use serde::{Serialize, Serializer};

use crate::{
error::{ReadError, WriteError},
Expand Down Expand Up @@ -55,6 +56,12 @@ impl<'list, T> IntoIterator for &'list DynamicList<T> {
}
}

impl<T: Serialize> Serialize for DynamicList<T> {
fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
serializer.collect_seq(self)
}
}

impl<T: SszSize> SszSize for DynamicList<T> {
const SIZE: Size = Size::Variable { minimum_size: 0 };
}
Expand Down
9 changes: 9 additions & 0 deletions types/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -737,6 +737,15 @@ impl Config {
}
}

#[must_use]
pub const fn blob_sidecar_subnet_count(&self, phase: Phase) -> Option<NonZeroU64> {
match phase {
Phase::Phase0 | Phase::Altair | Phase::Bellatrix | Phase::Capella => None,
Phase::Deneb => Some(self.blob_sidecar_subnet_count),
Phase::Electra => Some(self.blob_sidecar_subnet_count_electra),
}
}

fn fork_slots<P: Preset>(&self) -> impl Iterator<Item = (Phase, Toption<Slot>)> + '_ {
enum_iterator::all().map(|phase| (phase, self.fork_slot::<P>(phase)))
}
Expand Down
12 changes: 12 additions & 0 deletions types/src/nonstandard.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use smallvec::SmallVec;
use ssz::ContiguousList;
use static_assertions::assert_eq_size;
use strum::{AsRefStr, Display, EnumString};
use typenum::Unsigned as _;

use crate::{
altair::{
Expand Down Expand Up @@ -62,6 +63,17 @@ pub enum Phase {
Electra,
}

impl Phase {
#[must_use]
pub const fn max_blobs_per_block<P: Preset>(self) -> Option<u64> {
match self {
Self::Phase0 | Self::Altair | Self::Bellatrix | Self::Capella => None,
Self::Deneb => Some(P::MaxBlobsPerBlock::U64),
Self::Electra => Some(P::MaxBlobsPerBlockElectra::U64),
}
}
}

/// Like [`Option`], but with [`None`] greater than any [`Some`].
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
#[cfg_attr(test, derive(Debug))]
Expand Down

0 comments on commit db2c98f

Please sign in to comment.