Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix incorrect columns per subnet and config cleanup #5792

Merged
merged 3 commits into from
May 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion beacon_node/beacon_chain/src/data_availability_checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {
let custody_subnet_count = if import_all_data_columns {
T::EthSpec::data_column_subnet_count()
} else {
T::EthSpec::min_custody_requirement()
T::EthSpec::custody_requirement()
};

let custody_column_count =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ pub type DataColumnsToPublish<E> = Option<Vec<Arc<DataColumnSidecar<E>>>>;
pub struct PendingComponents<E: EthSpec> {
pub block_root: Hash256,
pub verified_blobs: FixedVector<Option<KzgVerifiedBlob<E>>, E::MaxBlobsPerBlock>,
pub verified_data_columns: VariableList<KzgVerifiedCustodyDataColumn<E>, E::DataColumnCount>,
pub verified_data_columns: VariableList<KzgVerifiedCustodyDataColumn<E>, E::NumberOfColumns>,
pub executed_block: Option<DietAvailabilityPendingExecutedBlock<E>>,
}

Expand Down
4 changes: 2 additions & 2 deletions beacon_node/beacon_chain/src/data_column_verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ impl<E: EthSpec> From<BeaconStateError> for GossipDataColumnError<E> {

pub type GossipVerifiedDataColumnList<T> = VariableList<
GossipVerifiedDataColumn<T>,
<<T as BeaconChainTypes>::EthSpec as EthSpec>::DataColumnCount,
<<T as BeaconChainTypes>::EthSpec as EthSpec>::NumberOfColumns,
>;

/// A wrapper around a `DataColumnSidecar` that indicates it has been approved for re-gossiping on
Expand Down Expand Up @@ -243,7 +243,7 @@ impl<E: EthSpec> KzgVerifiedDataColumn<E> {
}

pub type CustodyDataColumnList<E> =
VariableList<CustodyDataColumn<E>, <E as EthSpec>::DataColumnCount>;
VariableList<CustodyDataColumn<E>, <E as EthSpec>::NumberOfColumns>;

/// Data column that we must custody
#[derive(Debug, Derivative, Clone, Encode, Decode)]
Expand Down
4 changes: 2 additions & 2 deletions beacon_node/lighthouse_network/src/discovery/enr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ impl Eth2Enr for Enr {
fn custody_subnet_count<E: EthSpec>(&self) -> u64 {
self.get(PEERDAS_CUSTODY_SUBNET_COUNT_ENR_KEY)
.and_then(|custody_bytes| u64::from_ssz_bytes(custody_bytes).ok())
.unwrap_or(E::min_custody_requirement() as u64)
.unwrap_or(E::custody_requirement() as u64)
}

fn eth2(&self) -> Result<EnrForkId, &'static str> {
Expand Down Expand Up @@ -238,7 +238,7 @@ pub fn build_enr<E: EthSpec>(
let custody_subnet_count = if config.subscribe_all_data_column_subnets {
E::data_column_subnet_count() as u64
} else {
E::min_custody_requirement() as u64
E::custody_requirement() as u64
};

builder.add_value(
Expand Down
2 changes: 1 addition & 1 deletion beacon_node/lighthouse_network/src/types/globals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ mod test {
fn test_custody_count_default() {
let log = logging::test_logger();
let default_custody_requirement_column_count =
E::number_of_columns() / E::data_column_subnet_count() * E::min_custody_requirement();
E::number_of_columns() / E::data_column_subnet_count() * E::custody_requirement();
let globals = NetworkGlobals::<E>::new_test_globals(vec![], &log);
let any_epoch = Epoch::new(0);
let columns = globals.custody_columns(any_epoch).unwrap();
Expand Down
2 changes: 1 addition & 1 deletion beacon_node/network/src/sync/block_lookups/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1866,7 +1866,7 @@ fn custody_lookup_happy_path() {
// Should not request blobs
let id = r.expect_block_lookup_request(block.canonical_root());
r.complete_valid_block_request(id, block.into(), true);
let custody_column_count = E::min_custody_requirement() * E::data_columns_per_subnet();
let custody_column_count = E::custody_requirement() * E::data_columns_per_subnet();
let custody_ids = r.expect_only_data_columns_by_root_requests(block_root, custody_column_count);
r.complete_valid_custody_request(custody_ids, data_columns, false);
r.expect_no_active_lookups();
Expand Down
9 changes: 0 additions & 9 deletions consensus/types/presets/gnosis/deneb.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,3 @@ MAX_BLOB_COMMITMENTS_PER_BLOCK: 4096
MAX_BLOBS_PER_BLOCK: 6
# `floorlog2(BLOB_KZG_COMMITMENTS_GINDEX) + 1 + ceillog2(MAX_BLOB_COMMITMENTS_PER_BLOCK)` = 4 + 1 + 12 = 17
KZG_COMMITMENT_INCLUSION_PROOF_DEPTH: 17

# EIP-7594 (temporary in Deneb for the purpose of prototyping)
# ---------------------------------------------------------------
# `uint64(2**6)` (= 64)
FIELD_ELEMENTS_PER_CELL: 64
# uint64(floorlog2(get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments'))
KZG_COMMITMENTS_INCLUSION_PROOF_DEPTH: 4
# `uint64((FIELD_ELEMENTS_PER_BLOB * 2) // FIELD_ELEMENTS_PER_CELL)` (= 128)
NUMBER_OF_COLUMNS: 128
10 changes: 10 additions & 0 deletions consensus/types/presets/gnosis/eip7594.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# Mainnet preset - EIP7594

# Misc
# ---------------------------------------------------------------
# `uint64(2**6)` (= 64)
FIELD_ELEMENTS_PER_CELL: 64
# `uint64(2 * 4096)` (= 8192)
FIELD_ELEMENTS_PER_EXT_BLOB: 8192
# uint64(floorlog2(get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments'))
KZG_COMMITMENTS_INCLUSION_PROOF_DEPTH: 4
9 changes: 0 additions & 9 deletions consensus/types/presets/mainnet/deneb.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,3 @@ MAX_BLOB_COMMITMENTS_PER_BLOCK: 4096
MAX_BLOBS_PER_BLOCK: 6
# `floorlog2(get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments')) + 1 + ceillog2(MAX_BLOB_COMMITMENTS_PER_BLOCK)` = 4 + 1 + 12 = 17
KZG_COMMITMENT_INCLUSION_PROOF_DEPTH: 17

# EIP-7594 (temporary in Deneb for the purpose of prototyping)
# ---------------------------------------------------------------
# `uint64(2**6)` (= 64)
FIELD_ELEMENTS_PER_CELL: 64
# uint64(floorlog2(get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments'))
KZG_COMMITMENTS_INCLUSION_PROOF_DEPTH: 4
# `uint64((FIELD_ELEMENTS_PER_BLOB * 2) // FIELD_ELEMENTS_PER_CELL)` (= 128)
NUMBER_OF_COLUMNS: 128
10 changes: 10 additions & 0 deletions consensus/types/presets/mainnet/eip7594.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# Mainnet preset - EIP7594

# Misc
# ---------------------------------------------------------------
# `uint64(2**6)` (= 64)
FIELD_ELEMENTS_PER_CELL: 64
# `uint64(2 * 4096)` (= 8192)
FIELD_ELEMENTS_PER_EXT_BLOB: 8192
# uint64(floorlog2(get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments'))
KZG_COMMITMENTS_INCLUSION_PROOF_DEPTH: 4
9 changes: 0 additions & 9 deletions consensus/types/presets/minimal/deneb.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,3 @@ MAX_BLOB_COMMITMENTS_PER_BLOCK: 16
MAX_BLOBS_PER_BLOCK: 6
# [customized] `floorlog2(BLOB_KZG_COMMITMENTS_GINDEX) + 1 + ceillog2(MAX_BLOB_COMMITMENTS_PER_BLOCK)` = 4 + 1 + 4 = 9
KZG_COMMITMENT_INCLUSION_PROOF_DEPTH: 9

# EIP-7594 (temporary in Deneb for the purpose of prototyping)
# ---------------------------------------------------------------
# `uint64(2**6)` (= 64)
FIELD_ELEMENTS_PER_CELL: 64
# uint64(floorlog2(get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments'))
KZG_COMMITMENTS_INCLUSION_PROOF_DEPTH: 4
# `uint64((FIELD_ELEMENTS_PER_BLOB * 2) // FIELD_ELEMENTS_PER_CELL)` (= 128)
NUMBER_OF_COLUMNS: 128
10 changes: 10 additions & 0 deletions consensus/types/presets/minimal/eip7594.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# Minimal preset - EIP7594

# Misc
# ---------------------------------------------------------------
# `uint64(2**6)` (= 64)
FIELD_ELEMENTS_PER_CELL: 64
# `uint64(2 * 4096)` (= 8192)
FIELD_ELEMENTS_PER_EXT_BLOB: 8192
# uint64(floorlog2(get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments'))
KZG_COMMITMENTS_INCLUSION_PROOF_DEPTH: 4
4 changes: 2 additions & 2 deletions consensus/types/src/data_column_sidecar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -383,9 +383,9 @@ impl From<SszError> for DataColumnSidecarError {
}

pub type DataColumnSidecarList<E> =
VariableList<Arc<DataColumnSidecar<E>>, <E as EthSpec>::DataColumnCount>;
VariableList<Arc<DataColumnSidecar<E>>, <E as EthSpec>::NumberOfColumns>;
pub type FixedDataColumnSidecarList<E> =
FixedVector<Option<Arc<DataColumnSidecar<E>>>, <E as EthSpec>::DataColumnCount>;
FixedVector<Option<Arc<DataColumnSidecar<E>>>, <E as EthSpec>::NumberOfColumns>;

/// Converts a cell ssz List object to an array to be used with the kzg
/// crypto library.
Expand Down
68 changes: 41 additions & 27 deletions consensus/types/src/eth_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,17 +110,20 @@ pub trait EthSpec:
type MaxBlobsPerBlock: Unsigned + Clone + Sync + Send + Debug + PartialEq + Unpin;
type MaxBlobCommitmentsPerBlock: Unsigned + Clone + Sync + Send + Debug + PartialEq + Unpin;
type FieldElementsPerBlob: Unsigned + Clone + Sync + Send + Debug + PartialEq;
type FieldElementsPerCell: Unsigned + Clone + Sync + Send + Debug + PartialEq;
type BytesPerFieldElement: Unsigned + Clone + Sync + Send + Debug + PartialEq;
type KzgCommitmentInclusionProofDepth: Unsigned + Clone + Sync + Send + Debug + PartialEq;
/*
* New in PeerDAS
*/
type MinCustodyRequirement: Unsigned + Clone + Sync + Send + Debug + PartialEq;
type DataColumnSubnetCount: Unsigned + Clone + Sync + Send + Debug + PartialEq;
type DataColumnCount: Unsigned + Clone + Sync + Send + Debug + PartialEq;
type DataColumnsPerSubnet: Unsigned + Clone + Sync + Send + Debug + PartialEq;
type FieldElementsPerCell: Unsigned + Clone + Sync + Send + Debug + PartialEq;
type FieldElementsPerExtBlob: Unsigned + Clone + Sync + Send + Debug + PartialEq;
type KzgCommitmentsInclusionProofDepth: Unsigned + Clone + Sync + Send + Debug + PartialEq;
/*
* Config values in PeerDAS
*/
type CustodyRequirement: Unsigned + Clone + Sync + Send + Debug + PartialEq;
type DataColumnSidecarSubnetCount: Unsigned + Clone + Sync + Send + Debug + PartialEq;
type NumberOfColumns: Unsigned + Clone + Sync + Send + Debug + PartialEq;
/*
* Derived values (set these CAREFULLY)
*/
Expand Down Expand Up @@ -149,6 +152,11 @@ pub trait EthSpec:
/// Must be set to `BytesPerFieldElement * FieldElementsPerCell`.
type BytesPerCell: Unsigned + Clone + Sync + Send + Debug + PartialEq;

/// Number of data columns per subnet.
///
/// Must be set to `NumberOfColumns / DataColumnSidecarSubnetCount`
type DataColumnsPerSubnet: Unsigned + Clone + Sync + Send + Debug + PartialEq;

/*
* New in Electra
*/
Expand Down Expand Up @@ -296,6 +304,11 @@ pub trait EthSpec:
Self::FieldElementsPerBlob::to_usize()
}

/// Returns the `FIELD_ELEMENTS_PER_EXT_BLOB` constant for this specification.
fn field_elements_per_ext_blob() -> usize {
Self::FieldElementsPerExtBlob::to_usize()
}

/// Returns the `FIELD_ELEMENTS_PER_CELL` constant for this specification.
fn field_elements_per_cell() -> usize {
Self::FieldElementsPerCell::to_usize()
Expand Down Expand Up @@ -352,19 +365,19 @@ pub trait EthSpec:
}

fn number_of_columns() -> usize {
Self::DataColumnCount::to_usize()
Self::NumberOfColumns::to_usize()
}

fn data_columns_per_subnet() -> usize {
Self::DataColumnsPerSubnet::to_usize()
}

fn min_custody_requirement() -> usize {
Self::MinCustodyRequirement::to_usize()
fn custody_requirement() -> usize {
Self::CustodyRequirement::to_usize()
}

fn data_column_subnet_count() -> usize {
Self::DataColumnSubnetCount::to_usize()
Self::DataColumnSidecarSubnetCount::to_usize()
}

fn kzg_commitments_inclusion_proof_depth() -> usize {
Expand Down Expand Up @@ -414,13 +427,14 @@ impl EthSpec for MainnetEthSpec {
type BytesPerFieldElement = U32;
type FieldElementsPerBlob = U4096;
type FieldElementsPerCell = U64;
type FieldElementsPerExtBlob = U8192;
type BytesPerBlob = U131072;
type BytesPerCell = U2048;
type KzgCommitmentInclusionProofDepth = U17;
type MinCustodyRequirement = U4;
type DataColumnSubnetCount = U64;
type DataColumnCount = U128;
type DataColumnsPerSubnet = U4;
type CustodyRequirement = U4;
type DataColumnSidecarSubnetCount = U64;
type NumberOfColumns = U128;
type DataColumnsPerSubnet = U2;
type KzgCommitmentsInclusionProofDepth = U4; // inclusion of the whole list of commitments
type SyncSubcommitteeSize = U128; // 512 committee size / 4 sync committee subnet count
type MaxPendingAttestations = U4096; // 128 max attestations * 32 slots per epoch
Expand Down Expand Up @@ -461,20 +475,20 @@ impl EthSpec for MinimalEthSpec {
type SlotsPerEth1VotingPeriod = U32; // 4 epochs * 8 slots per epoch
type MaxWithdrawalsPerPayload = U4;
type FieldElementsPerBlob = U4096;
type FieldElementsPerCell = U64;
type BytesPerBlob = U131072;
type BytesPerCell = U2048;
type MaxBlobCommitmentsPerBlock = U16;
type KzgCommitmentInclusionProofDepth = U9;
type PendingPartialWithdrawalsLimit = U64;
type PendingConsolidationsLimit = U64;
type MaxDepositReceiptsPerPayload = U4;
type MaxWithdrawalRequestsPerPayload = U2;
// DAS spec values copied from `MainnetEthSpec`
type MinCustodyRequirement = U4;
type DataColumnSubnetCount = U64;
type DataColumnCount = U128;
type DataColumnsPerSubnet = U4;
type FieldElementsPerCell = U64;
type FieldElementsPerExtBlob = U8192;
type BytesPerCell = U2048;
type CustodyRequirement = U4;
type DataColumnSidecarSubnetCount = U64;
type NumberOfColumns = U128;
type DataColumnsPerSubnet = U2;
type KzgCommitmentsInclusionProofDepth = U4;

params_from_eth_spec!(MainnetEthSpec {
Expand Down Expand Up @@ -551,10 +565,8 @@ impl EthSpec for GnosisEthSpec {
type MaxBlobsPerBlock = U6;
type MaxBlobCommitmentsPerBlock = U4096;
type FieldElementsPerBlob = U4096;
type FieldElementsPerCell = U64;
type BytesPerFieldElement = U32;
type BytesPerBlob = U131072;
type BytesPerCell = U2048;
type KzgCommitmentInclusionProofDepth = U17;
type PendingBalanceDepositsLimit = U134217728;
type PendingPartialWithdrawalsLimit = U134217728;
Expand All @@ -564,11 +576,13 @@ impl EthSpec for GnosisEthSpec {
type MaxAttesterSlashingsElectra = U1;
type MaxAttestationsElectra = U8;
type MaxWithdrawalRequestsPerPayload = U16;
// DAS spec values copied from `MainnetEthSpec`
type MinCustodyRequirement = U4;
type DataColumnSubnetCount = U64;
type DataColumnCount = U128;
type DataColumnsPerSubnet = U4;
type FieldElementsPerCell = U64;
type FieldElementsPerExtBlob = U8192;
type BytesPerCell = U2048;
type CustodyRequirement = U4;
type DataColumnSidecarSubnetCount = U64;
type NumberOfColumns = U128;
type DataColumnsPerSubnet = U2;
type KzgCommitmentsInclusionProofDepth = U4;

fn default_spec() -> ChainSpec {
Expand Down
36 changes: 25 additions & 11 deletions consensus/types/src/preset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,13 +214,6 @@ pub struct DenebPreset {
pub max_blob_commitments_per_block: u64,
#[serde(with = "serde_utils::quoted_u64")]
pub field_elements_per_blob: u64,
// EIP-7594 DAS presets - to be moved to the next fork
#[serde(with = "serde_utils::quoted_u64")]
pub field_elements_per_cell: u64,
#[serde(with = "serde_utils::quoted_u64")]
pub kzg_commitments_inclusion_proof_depth: u64,
#[serde(with = "serde_utils::quoted_u64")]
pub number_of_columns: u64,
}

impl DenebPreset {
Expand All @@ -229,10 +222,6 @@ impl DenebPreset {
max_blobs_per_block: E::max_blobs_per_block() as u64,
max_blob_commitments_per_block: E::max_blob_commitments_per_block() as u64,
field_elements_per_blob: E::field_elements_per_blob() as u64,
field_elements_per_cell: E::field_elements_per_cell() as u64,
kzg_commitments_inclusion_proof_depth: E::kzg_commitments_inclusion_proof_depth()
as u64,
number_of_columns: E::number_of_columns() as u64,
}
}
}
Expand Down Expand Up @@ -289,6 +278,28 @@ impl ElectraPreset {
}
}

#[derive(Debug, PartialEq, Clone, Serialize, Deserialize)]
#[serde(rename_all = "UPPERCASE")]
pub struct Eip7594Preset {
#[serde(with = "serde_utils::quoted_u64")]
pub field_elements_per_cell: u64,
#[serde(with = "serde_utils::quoted_u64")]
pub field_elements_per_ext_blob: u64,
#[serde(with = "serde_utils::quoted_u64")]
pub kzg_commitments_inclusion_proof_depth: u64,
}

impl Eip7594Preset {
pub fn from_chain_spec<E: EthSpec>(_spec: &ChainSpec) -> Self {
Self {
field_elements_per_cell: E::field_elements_per_cell() as u64,
field_elements_per_ext_blob: E::field_elements_per_ext_blob() as u64,
kzg_commitments_inclusion_proof_depth: E::kzg_commitments_inclusion_proof_depth()
as u64,
}
}
}

#[cfg(test)]
mod test {
use super::*;
Expand Down Expand Up @@ -333,6 +344,9 @@ mod test {

let electra: ElectraPreset = preset_from_file(&preset_name, "electra.yaml");
assert_eq!(electra, ElectraPreset::from_chain_spec::<E>(&spec));

let eip7594: Eip7594Preset = preset_from_file(&preset_name, "eip7594.yaml");
assert_eq!(eip7594, Eip7594Preset::from_chain_spec::<E>(&spec));
}

#[test]
Expand Down
Loading