Fix data columns sorting when reconstructing blobs (#8510)

Closes #8509


  


Co-Authored-By: Antoine James <[email protected]>

Author: 0xMushow

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -1248,7 +1248,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                 let num_required_columns = T::EthSpec::number_of_columns() / 2;
                 let reconstruction_possible = columns.len() >= num_required_columns;
                 if reconstruction_possible {
-                    reconstruct_blobs(&self.kzg, &columns, None, &block, &self.spec)
+                    reconstruct_blobs(&self.kzg, columns, None, &block, &self.spec)
                         .map(Some)
                         .map_err(Error::FailedToReconstructBlobs)
                 } else {

beacon_node/beacon_chain/src/kzg_utils.rs

@@ -308,12 +308,14 @@ pub(crate) fn build_data_column_sidecars<E: EthSpec>(
 /// and it will be slow if the node needs to reconstruct the blobs
 pub fn reconstruct_blobs<E: EthSpec>(
     kzg: &Kzg,
-    data_columns: &[Arc<DataColumnSidecar<E>>],
+    mut data_columns: Vec<Arc<DataColumnSidecar<E>>>,
     blob_indices_opt: Option<Vec<u64>>,
     signed_block: &SignedBlindedBeaconBlock<E>,
     spec: &ChainSpec,
 ) -> Result<BlobSidecarList<E>, String> {
-    // The data columns are from the database, so we assume their correctness.
+    // 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("data_columns should have at least one element".to_string())?;
@@ -331,7 +333,7 @@ pub fn reconstruct_blobs<E: EthSpec>(
         .map(|row_index| {
             let mut cells: Vec<KzgCellRef> = vec![];
             let mut cell_ids: Vec<u64> = vec![];
-            for data_column in data_columns {
+            for data_column in &data_columns {
                 let cell = data_column
                     .column
                     .get(row_index)
@@ -463,6 +465,7 @@ mod test {
         test_reconstruct_data_columns(&kzg, &spec);
         test_reconstruct_data_columns_unordered(&kzg, &spec);
         test_reconstruct_blobs_from_data_columns(&kzg, &spec);
+        test_reconstruct_blobs_from_data_columns_unordered(&kzg, &spec);
         test_validate_data_columns(&kzg, &spec);
     }
 
@@ -595,7 +598,7 @@ mod test {
         let blob_indices = vec![1, 2];
         let reconstructed_blobs = reconstruct_blobs(
             kzg,
-            &column_sidecars.iter().as_slice()[0..column_sidecars.len() / 2],
+            column_sidecars[0..column_sidecars.len() / 2].to_vec(),
             Some(blob_indices.clone()),
             &signed_blinded_block,
             spec,
@@ -613,6 +616,31 @@ mod test {
         }
     }
 
+    #[track_caller]
+    fn test_reconstruct_blobs_from_data_columns_unordered(kzg: &Kzg, spec: &ChainSpec) {
+        let num_of_blobs = 2;
+        let (signed_block, blobs, proofs) =
+            create_test_fulu_block_and_blobs::<E>(num_of_blobs, spec);
+        let blob_refs = blobs.iter().collect::<Vec<_>>();
+        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_blobs
+
+        let signed_blinded_block = signed_block.into();
+        let reconstructed_blobs =
+            reconstruct_blobs(kzg, subset_columns, None, &signed_blinded_block, spec).unwrap();
+
+        for (i, original_blob) in blobs.iter().enumerate() {
+            let reconstructed_blob = &reconstructed_blobs.get(i).unwrap().blob;
+            assert_eq!(reconstructed_blob, original_blob, "{i}");
+        }
+    }
+
     fn get_kzg() -> Kzg {
         Kzg::new_from_trusted_setup(&get_trusted_setup()).expect("should create kzg")
     }

beacon_node/http_api/src/block_id.rs

@@ -474,7 +474,7 @@ impl BlockId {
                 )
                 .collect::<Result<Vec<_>, _>>()?;
 
-            reconstruct_blobs(&chain.kzg, &data_columns, blob_indices, block, &chain.spec).map_err(
+            reconstruct_blobs(&chain.kzg, data_columns, blob_indices, block, &chain.spec).map_err(
                 |e| {
                     warp_utils::reject::custom_server_error(format!(
                         "Error reconstructing data columns: {e:?}"