diff --git a/beacon_node/beacon_chain/benches/benches.rs b/beacon_node/beacon_chain/benches/benches.rs index 5c56594317..d090fc35f7 100644 --- a/beacon_node/beacon_chain/benches/benches.rs +++ b/beacon_node/beacon_chain/benches/benches.rs @@ -55,7 +55,7 @@ fn all_benches(c: &mut Criterion) { b.iter(|| { black_box(reconstruct_data_columns( &kzg, - &column_sidecars.iter().as_slice()[0..column_sidecars.len() / 2], + column_sidecars.iter().as_slice()[0..column_sidecars.len() / 2].to_vec(), spec.as_ref(), )) }) diff --git a/beacon_node/beacon_chain/src/data_column_verification.rs b/beacon_node/beacon_chain/src/data_column_verification.rs index fb88db1300..bc7778cc63 100644 --- a/beacon_node/beacon_chain/src/data_column_verification.rs +++ b/beacon_node/beacon_chain/src/data_column_verification.rs @@ -376,7 +376,7 @@ impl KzgVerifiedCustodyDataColumn { ) -> Result>, KzgError> { let all_data_columns = reconstruct_data_columns( kzg, - &partial_set_of_columns + partial_set_of_columns .iter() .map(|d| d.clone_arc()) .collect::>(), diff --git a/beacon_node/beacon_chain/src/kzg_utils.rs b/beacon_node/beacon_chain/src/kzg_utils.rs index 3063e78337..2147ed5966 100644 --- a/beacon_node/beacon_chain/src/kzg_utils.rs +++ b/beacon_node/beacon_chain/src/kzg_utils.rs @@ -365,14 +365,18 @@ pub fn reconstruct_blobs( /// Reconstruct all data columns from a subset of data column sidecars (requires at least 50%). pub fn reconstruct_data_columns( kzg: &Kzg, - data_columns: &[Arc>], + mut data_columns: Vec>>, spec: &ChainSpec, ) -> Result, KzgError> { + // Sort data columns by index to ensure ascending order for KZG operations + data_columns.sort_unstable_by_key(|dc| dc.index); + let first_data_column = data_columns .first() .ok_or(KzgError::InconsistentArrayLength( "data_columns should have at least one element".to_string(), ))?; + let num_of_blobs = first_data_column.kzg_commitments.len(); let blob_cells_and_proofs_vec = @@ -381,7 +385,7 @@ pub fn reconstruct_data_columns( .map(|row_index| { let mut cells: Vec = vec![]; let mut cell_ids: Vec = vec![]; - for data_column in data_columns { + for data_column in &data_columns { let cell = data_column.column.get(row_index).ok_or( KzgError::InconsistentArrayLength(format!( "Missing data column at row index {row_index}" @@ -433,6 +437,7 @@ mod test { test_build_data_columns_empty(&kzg, &spec); test_build_data_columns(&kzg, &spec); test_reconstruct_data_columns(&kzg, &spec); + test_reconstruct_data_columns_unordered(&kzg, &spec); test_reconstruct_blobs_from_data_columns(&kzg, &spec); test_validate_data_columns(&kzg, &spec); } @@ -505,7 +510,7 @@ mod test { #[track_caller] fn test_reconstruct_data_columns(kzg: &Kzg, spec: &ChainSpec) { - let num_of_blobs = 6; + let num_of_blobs = 2; let (signed_block, blobs, proofs) = create_test_fulu_block_and_blobs::(num_of_blobs, spec); let blob_refs = blobs.iter().collect::>(); @@ -516,7 +521,7 @@ mod test { // Now reconstruct let reconstructed_columns = reconstruct_data_columns( kzg, - &column_sidecars.iter().as_slice()[0..column_sidecars.len() / 2], + column_sidecars.iter().as_slice()[0..column_sidecars.len() / 2].to_vec(), spec, ) .unwrap(); @@ -526,6 +531,27 @@ mod test { } } + #[track_caller] + fn test_reconstruct_data_columns_unordered(kzg: &Kzg, spec: &ChainSpec) { + let num_of_blobs = 2; + let (signed_block, blobs, proofs) = + create_test_fulu_block_and_blobs::(num_of_blobs, spec); + let blob_refs = blobs.iter().collect::>(); + let column_sidecars = + blobs_to_data_column_sidecars(&blob_refs, proofs.to_vec(), &signed_block, kzg, spec) + .unwrap(); + + // Test reconstruction with columns in reverse order (non-ascending) + let mut subset_columns: Vec<_> = + column_sidecars.iter().as_slice()[0..column_sidecars.len() / 2].to_vec(); + subset_columns.reverse(); // This would fail without proper sorting in reconstruct_data_columns + let reconstructed_columns = reconstruct_data_columns(kzg, subset_columns, spec).unwrap(); + + for i in 0..E::number_of_columns() { + assert_eq!(reconstructed_columns.get(i), column_sidecars.get(i), "{i}"); + } + } + #[track_caller] fn test_reconstruct_blobs_from_data_columns(kzg: &Kzg, spec: &ChainSpec) { let num_of_blobs = 6;