Skip to content

Commit

Permalink
Make max_blobs_per_block a config parameter (#6329)
Browse files Browse the repository at this point in the history
Squashed commit of the following:

commit 04b3743
Author: Michael Sproul <[email protected]>
Date:   Mon Jan 6 17:36:58 2025 +1100

    Add test

commit 440e854
Author: Michael Sproul <[email protected]>
Date:   Mon Jan 6 17:24:50 2025 +1100

    Move RuntimeFixedVector into module and rename

commit f66e179
Author: Michael Sproul <[email protected]>
Date:   Mon Jan 6 17:17:17 2025 +1100

    Fix release tests

commit e4bfe71
Author: Michael Sproul <[email protected]>
Date:   Mon Jan 6 17:05:30 2025 +1100

    Thread through ChainSpec

commit 063b79c
Author: Michael Sproul <[email protected]>
Date:   Mon Jan 6 15:32:16 2025 +1100

    Try fixing tests

commit 88bedf0
Author: Michael Sproul <[email protected]>
Date:   Mon Jan 6 15:04:37 2025 +1100

    Revert "Remove footgun function"

    This reverts commit de01f92.

commit 32483d3
Author: Michael Sproul <[email protected]>
Date:   Mon Jan 6 15:04:32 2025 +1100

    Fix typo

commit 2e86585
Author: Michael Sproul <[email protected]>
Date:   Mon Jan 6 15:04:15 2025 +1100

    Move from preset to config

commit 1095d60
Author: Michael Sproul <[email protected]>
Date:   Mon Jan 6 14:38:40 2025 +1100

    Minor simplifications

commit de01f92
Author: Michael Sproul <[email protected]>
Date:   Mon Jan 6 14:06:57 2025 +1100

    Remove footgun function

commit 0c2c8c4
Merge: 21ecb58 f51a292
Author: Michael Sproul <[email protected]>
Date:   Mon Jan 6 14:02:50 2025 +1100

    Merge remote-tracking branch 'origin/unstable' into max-blobs-preset

commit f51a292
Author: Daniel Knopik <[email protected]>
Date:   Fri Jan 3 20:27:21 2025 +0100

    fully lint only explicitly to avoid unnecessary rebuilds (#6753)

    * fully lint only explicitly to avoid unnecessary rebuilds

commit 7e0cdde
Author: Akihito Nakano <[email protected]>
Date:   Tue Dec 24 10:38:56 2024 +0900

    Make sure we have fanout peers when publish (#6738)

    * Ensure that `fanout_peers` is always non-empty if it's `Some`

commit 21ecb58
Merge: 2fcb293 9aefb55
Author: Pawan Dhananjay <[email protected]>
Date:   Mon Oct 21 14:46:00 2024 -0700

    Merge branch 'unstable' into max-blobs-preset

commit 2fcb293
Author: Pawan Dhananjay <[email protected]>
Date:   Fri Sep 6 18:28:31 2024 -0700

    Fix test from unstable

commit 12c6ef1
Author: Pawan Dhananjay <[email protected]>
Date:   Wed Sep 4 16:16:36 2024 -0700

    Fix some more tests

commit d37733b
Author: Pawan Dhananjay <[email protected]>
Date:   Wed Sep 4 12:47:36 2024 -0700

    Fix test compilations

commit 52bb581
Author: Pawan Dhananjay <[email protected]>
Date:   Tue Sep 3 18:38:19 2024 -0700

    cleanup

commit e71020e
Author: Pawan Dhananjay <[email protected]>
Date:   Tue Sep 3 17:16:10 2024 -0700

    Fix take impl on RuntimeFixedList

commit 13f9bba
Merge: 60100fc 4e675cf
Author: Pawan Dhananjay <[email protected]>
Date:   Tue Sep 3 16:08:59 2024 -0700

    Merge branch 'unstable' into max-blobs-preset

commit 60100fc
Author: Pawan Dhananjay <[email protected]>
Date:   Fri Aug 30 16:04:11 2024 -0700

    Fix some todos

commit a9cb329
Author: Pawan Dhananjay <[email protected]>
Date:   Fri Aug 30 15:54:00 2024 -0700

    Use empty_uninitialized and fix warnings

commit 4dc6e65
Author: Pawan Dhananjay <[email protected]>
Date:   Fri Aug 30 15:53:18 2024 -0700

    Add restrictions to RuntimeVariableList api

commit 25feedf
Author: Pawan Dhananjay <[email protected]>
Date:   Thu Aug 29 16:11:19 2024 -0700

    First pass
  • Loading branch information
michaelsproul committed Jan 7, 2025
1 parent 2dbd3b7 commit 3b788bf
Show file tree
Hide file tree
Showing 51 changed files with 559 additions and 263 deletions.
1 change: 1 addition & 0 deletions .cargo/config.toml
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
[env]
# Set the number of arenas to 16 when using jemalloc.
JEMALLOC_SYS_WITH_MALLOC_CONF = "abort_conf:true,narenas:16"

2 changes: 1 addition & 1 deletion .github/workflows/test-suite.yml
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ jobs:
- name: Check formatting with cargo fmt
run: make cargo-fmt
- name: Lint code for quality and style with Clippy
run: make lint
run: make lint-full
- name: Certify Cargo.lock freshness
run: git diff --exit-code Cargo.lock
- name: Typecheck benchmark code without running it
Expand Down
6 changes: 5 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ test-full: cargo-fmt test-release test-debug test-ef test-exec-engine
# Lints the code for bad style and potentially unsafe arithmetic using Clippy.
# Clippy lints are opt-in per-crate for now. By default, everything is allowed except for performance and correctness lints.
lint:
RUSTFLAGS="-C debug-assertions=no $(RUSTFLAGS)" cargo clippy --workspace --benches --tests $(EXTRA_CLIPPY_OPTS) --features "$(TEST_FEATURES)" -- \
cargo clippy --workspace --benches --tests $(EXTRA_CLIPPY_OPTS) --features "$(TEST_FEATURES)" -- \
-D clippy::fn_to_numeric_cast_any \
-D clippy::manual_let_else \
-D clippy::large_stack_frames \
Expand All @@ -220,6 +220,10 @@ lint:
lint-fix:
EXTRA_CLIPPY_OPTS="--fix --allow-staged --allow-dirty" $(MAKE) lint

# Also run the lints on the optimized-only tests
lint-full:
RUSTFLAGS="-C debug-assertions=no $(RUSTFLAGS)" $(MAKE) lint

# Runs the makefile in the `ef_tests` repo.
#
# May download and extract an archive of test vectors from the ethereum
Expand Down
2 changes: 1 addition & 1 deletion beacon_node/beacon_chain/src/beacon_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1243,7 +1243,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
pub fn get_blobs(&self, block_root: &Hash256) -> Result<BlobSidecarList<T::EthSpec>, Error> {
match self.store.get_blobs(block_root)? {
Some(blobs) => Ok(blobs),
None => Ok(BlobSidecarList::default()),
None => Ok(BlobSidecarList::empty_uninitialized()),
}
}

Expand Down
2 changes: 1 addition & 1 deletion beacon_node/beacon_chain/src/blob_verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ pub fn validate_blob_sidecar_for_gossip<T: BeaconChainTypes, O: ObservationStrat
// since we only subscribe to `MaxBlobsPerBlock` subnets over gossip network.
// We include this check only for completeness.
// Getting this error would imply something very wrong with our networking decoding logic.
if blob_index >= T::EthSpec::max_blobs_per_block() as u64 {
if blob_index >= chain.spec.max_blobs_per_block(blob_epoch) {
return Err(GossipBlobError::InvalidSubnet {
expected: subnet,
received: blob_index,
Expand Down
20 changes: 1 addition & 19 deletions beacon_node/beacon_chain/src/block_verification_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,10 @@ use crate::data_column_verification::{CustodyDataColumn, CustodyDataColumnList};
use crate::eth1_finalization_cache::Eth1FinalizationData;
use crate::{get_block_root, PayloadVerificationOutcome};
use derivative::Derivative;
use ssz_types::VariableList;
use state_processing::ConsensusContext;
use std::fmt::{Debug, Formatter};
use std::sync::Arc;
use types::blob_sidecar::{BlobIdentifier, FixedBlobSidecarList};
use types::blob_sidecar::BlobIdentifier;
use types::{
BeaconBlockRef, BeaconState, BlindedPayload, BlobSidecarList, ChainSpec, Epoch, EthSpec,
Hash256, RuntimeVariableList, SignedBeaconBlock, SignedBeaconBlockHeader, Slot,
Expand Down Expand Up @@ -176,23 +175,6 @@ impl<E: EthSpec> RpcBlock<E> {
})
}

pub fn new_from_fixed(
block_root: Hash256,
block: Arc<SignedBeaconBlock<E>>,
blobs: FixedBlobSidecarList<E>,
) -> Result<Self, AvailabilityCheckError> {
let filtered = blobs
.into_iter()
.filter_map(|b| b.clone())
.collect::<Vec<_>>();
let blobs = if filtered.is_empty() {
None
} else {
Some(VariableList::from(filtered))
};
Self::new(Some(block_root), block, blobs)
}

#[allow(clippy::type_complexity)]
pub fn deconstruct(
self,
Expand Down
14 changes: 8 additions & 6 deletions beacon_node/beacon_chain/src/data_availability_checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,9 +215,12 @@ impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {
// Note: currently not reporting which specific blob is invalid because we fetch all blobs
// from the same peer for both lookup and range sync.

let verified_blobs =
KzgVerifiedBlobList::new(blobs.iter().flatten().cloned(), &self.kzg, seen_timestamp)
.map_err(AvailabilityCheckError::InvalidBlobs)?;
let verified_blobs = KzgVerifiedBlobList::new(
blobs.into_vec().into_iter().flatten(),
&self.kzg,
seen_timestamp,
)
.map_err(AvailabilityCheckError::InvalidBlobs)?;

self.availability_cache
.put_kzg_verified_blobs(block_root, verified_blobs, &self.log)
Expand Down Expand Up @@ -400,14 +403,13 @@ impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {
blocks: Vec<RpcBlock<T::EthSpec>>,
) -> Result<Vec<MaybeAvailableBlock<T::EthSpec>>, AvailabilityCheckError> {
let mut results = Vec::with_capacity(blocks.len());
let all_blobs: BlobSidecarList<T::EthSpec> = blocks
let all_blobs = blocks
.iter()
.filter(|block| self.blobs_required_for_block(block.as_block()))
// this clone is cheap as it's cloning an Arc
.filter_map(|block| block.blobs().cloned())
.flatten()
.collect::<Vec<_>>()
.into();
.collect::<Vec<_>>();

// verify kzg for all blobs at once
if !all_blobs.is_empty() {
Expand Down
Loading

0 comments on commit 3b788bf

Please sign in to comment.