From 80892e634d16814d0d63e1fa9f337cb40327c0fe Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Thu, 16 May 2024 00:35:38 +0300 Subject: [PATCH] Fix DAS branch CI (#5793) * Fix invalid syntax. * Update cli doc. Ignore get_custody_columns test temporarily. * Fix failing test and add verify inclusion test. * Undo accidentally removed code. --- beacon_node/beacon_chain/src/test_utils.rs | 24 +++++-------------- .../beacon_chain/tests/block_verification.rs | 4 ++-- .../network/src/sync/block_lookups/tests.rs | 14 ++++++----- .../src/sync/block_sidecar_coupling.rs | 2 +- beacon_node/src/cli.rs | 2 ++ beacon_node/src/config.rs | 6 +++++ book/src/help_bn.md | 5 +--- consensus/types/src/data_column_sidecar.rs | 1 + testing/ef_tests/check_all_files_accessed.py | 2 ++ testing/ef_tests/tests/tests.rs | 2 ++ 10 files changed, 31 insertions(+), 31 deletions(-) diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index b7db2462ca9..aa7ecc054e4 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -2578,32 +2578,20 @@ pub fn generate_rand_block_and_blobs( (block, blob_sidecars) } +#[allow(clippy::type_complexity)] pub fn generate_rand_block_and_data_columns( fork_name: ForkName, num_blobs: NumBlobs, rng: &mut impl Rng, ) -> ( SignedBeaconBlock>, - Vec>, + Vec>>, ) { let (block, blobs) = generate_rand_block_and_blobs(fork_name, num_blobs, rng); - let blob = blobs.first().expect("should have at least 1 blob"); - - let data_columns = (0..E::number_of_columns()) - .map(|index| DataColumnSidecar { - index: index as u64, - column: <_>::default(), - kzg_commitments: block - .message() - .body() - .blob_kzg_commitments() - .unwrap() - .clone(), - kzg_proofs: (vec![]).into(), - signed_block_header: blob.signed_block_header.clone(), - kzg_commitments_inclusion_proof: <_>::default(), - }) - .collect::>(); + let blob: BlobsList = blobs.into_iter().map(|b| b.blob).collect::>().into(); + let data_columns = DataColumnSidecar::build_sidecars(&blob, &block, &KZG) + .unwrap() + .into(); (block, data_columns) } diff --git a/beacon_node/beacon_chain/tests/block_verification.rs b/beacon_node/beacon_chain/tests/block_verification.rs index c1b7261fc21..92a873ff3bc 100644 --- a/beacon_node/beacon_chain/tests/block_verification.rs +++ b/beacon_node/beacon_chain/tests/block_verification.rs @@ -1362,7 +1362,7 @@ async fn add_base_block_to_altair_chain() { ) .await, ChainSegmentResult::Failed { - .. + imported_blocks: _, error: BlockError::InconsistentFork(InconsistentFork { fork_at_slot: ForkName::Altair, object_fork: ForkName::Base, @@ -1497,7 +1497,7 @@ async fn add_altair_block_to_base_chain() { ) .await, ChainSegmentResult::Failed { - .. + imported_blocks: _, error: BlockError::InconsistentFork(InconsistentFork { fork_at_slot: ForkName::Base, object_fork: ForkName::Altair, diff --git a/beacon_node/network/src/sync/block_lookups/tests.rs b/beacon_node/network/src/sync/block_lookups/tests.rs index 173ae761f65..222d58604da 100644 --- a/beacon_node/network/src/sync/block_lookups/tests.rs +++ b/beacon_node/network/src/sync/block_lookups/tests.rs @@ -241,7 +241,9 @@ impl TestRig { generate_rand_block_and_blobs::(fork_name, num_blobs, rng) } - fn rand_block_and_data_columns(&mut self) -> (SignedBeaconBlock, Vec>) { + fn rand_block_and_data_columns( + &mut self, + ) -> (SignedBeaconBlock, Vec>>) { let num_blobs = NumBlobs::Number(1); generate_rand_block_and_data_columns::(self.fork_name, num_blobs, &mut self.rng) } @@ -640,7 +642,7 @@ impl TestRig { fn complete_valid_sampling_column_requests( &mut self, sampling_ids: SamplingIds, - data_columns: Vec>, + data_columns: Vec>>, ) { for (id, column_index) in sampling_ids { self.log(&format!("return valid data column for {column_index}")); @@ -654,7 +656,7 @@ impl TestRig { fn complete_valid_sampling_column_request( &mut self, id: DataColumnsByRootRequestId, - data_column: DataColumnSidecar, + data_column: Arc>, ) { let block_root = data_column.block_root(); let column_index = data_column.index; @@ -677,7 +679,7 @@ impl TestRig { fn complete_valid_custody_request( &mut self, sampling_ids: SamplingIds, - data_columns: Vec>, + data_columns: Vec>>, missing_components: bool, ) { let lookup_id = if let DataColumnsByRootRequester::Custody(id) = @@ -720,14 +722,14 @@ impl TestRig { fn complete_data_columns_by_root_request( &mut self, id: DataColumnsByRootRequestId, - data_column: DataColumnSidecar, + data_column: Arc>, ) { let peer_id = PeerId::random(); // Send chunk self.send_sync_message(SyncMessage::RpcDataColumn { request_id: SyncRequestId::DataColumnsByRoot(id), peer_id, - data_column: Some(Arc::new(data_column)), + data_column: Some(data_column), seen_timestamp: timestamp_now(), }); // Send stream termination diff --git a/beacon_node/network/src/sync/block_sidecar_coupling.rs b/beacon_node/network/src/sync/block_sidecar_coupling.rs index ca6460f4305..f2f22d1ecc1 100644 --- a/beacon_node/network/src/sync/block_sidecar_coupling.rs +++ b/beacon_node/network/src/sync/block_sidecar_coupling.rs @@ -299,7 +299,7 @@ mod tests { for block in &blocks { for column in &block.1 { if expects_custody_columns.contains(&column.index) { - info.add_data_column(Some(column.clone().into())); + info.add_data_column(Some(column.clone())); } } } diff --git a/beacon_node/src/cli.rs b/beacon_node/src/cli.rs index d4a733f8ef9..e44910e0c0a 100644 --- a/beacon_node/src/cli.rs +++ b/beacon_node/src/cli.rs @@ -47,9 +47,11 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { .takes_value(false), ) .arg( + // TODO(das): remove this before release Arg::with_name("malicious-withhold-count") .long("malicious-withhold-count") .help("TESTING ONLY do not use this") + .hidden(true) .takes_value(true), ) .arg( diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs index 3211c26d76d..f02b742f280 100644 --- a/beacon_node/src/config.rs +++ b/beacon_node/src/config.rs @@ -443,6 +443,12 @@ pub fn get_config( client_config.store.epochs_per_blob_prune = epochs_per_blob_prune; } + if let Some(blob_prune_margin_epochs) = + clap_utils::parse_optional(cli_args, "blob-prune-margin-epochs")? + { + client_config.store.blob_prune_margin_epochs = blob_prune_margin_epochs; + } + if let Some(malicious_withhold_count) = clap_utils::parse_optional(cli_args, "malicious-withhold-count")? { diff --git a/book/src/help_bn.md b/book/src/help_bn.md index 89d037d48ae..894369ca9f6 100644 --- a/book/src/help_bn.md +++ b/book/src/help_bn.md @@ -259,7 +259,7 @@ OPTIONS: --graffiti Specify your custom graffiti to be included in blocks. Defaults to the current version and commit, truncated - to fit in 32 bytes. + to fit in 32 bytes. --historic-state-cache-size Specifies how many states from the freezer database should cache in memory [default: 1] @@ -324,9 +324,6 @@ OPTIONS: --logfile-max-size The maximum size (in MB) each log file can grow to before rotating. If set to 0, background file logging is disabled. [default: 200] - --malicious-withhold-count - TESTING ONLY do not use this - --max-skip-slots Refuse to skip more than this many slots when processing an attestation. This prevents nodes on minority forks from wasting our time and disk space, but could also cause unnecessary consensus failures, so is diff --git a/consensus/types/src/data_column_sidecar.rs b/consensus/types/src/data_column_sidecar.rs index c6183414c5d..75687a43dfa 100644 --- a/consensus/types/src/data_column_sidecar.rs +++ b/consensus/types/src/data_column_sidecar.rs @@ -461,6 +461,7 @@ mod test { col_sidecar.kzg_commitments_inclusion_proof, block_kzg_commitments_inclusion_proof ); + assert!(col_sidecar.verify_inclusion_proof()); } } diff --git a/testing/ef_tests/check_all_files_accessed.py b/testing/ef_tests/check_all_files_accessed.py index 8c7891a9030..67cdbe9b969 100755 --- a/testing/ef_tests/check_all_files_accessed.py +++ b/testing/ef_tests/check_all_files_accessed.py @@ -22,6 +22,8 @@ excluded_paths = [ # TODO(das): remove once electra tests are on unstable "tests/.*/electra/", + # TODO(das): ignore until new spec test release with column subnet count = 64. + "tests/.*/.*/.*/get_custody_columns/", # Eth1Block and PowBlock # # Intentionally omitted, as per https://github.com/sigp/lighthouse/issues/1835 diff --git a/testing/ef_tests/tests/tests.rs b/testing/ef_tests/tests/tests.rs index 11c504c75b0..8775c6d3bd2 100644 --- a/testing/ef_tests/tests/tests.rs +++ b/testing/ef_tests/tests/tests.rs @@ -775,7 +775,9 @@ fn rewards() { } } +// TODO(das): ignore until new spec test release with column subnet count = 64. #[test] +#[ignore] fn get_custody_columns() { GetCustodyColumnsHandler::::default() .run_for_feature(ForkName::Deneb, FeatureName::Eip7594);