From cd17f63d818fc5f403678f670a9384579a5cfb9c Mon Sep 17 00:00:00 2001 From: behzad nouri Date: Sun, 5 Dec 2021 14:42:09 +0000 Subject: [PATCH] adds back position field to coding-shred-header (#21600) https://github.com/solana-labs/solana/pull/17004 removed position field from coding-shred-header because as it stands the field is redundant and unused. However, with the upcoming changes to erasure coding schema this field will no longer be redundant and needs to be populated. --- core/src/retransmit_stage.rs | 6 ++--- core/src/window_service.rs | 20 +++++++++++++++-- ledger/src/blockstore.rs | 18 +++++++++++++-- ledger/src/shred.rs | 43 +++++++++++++++++++++++++++--------- 4 files changed, 69 insertions(+), 18 deletions(-) diff --git a/core/src/retransmit_stage.rs b/core/src/retransmit_stage.rs index dacf89e06..fecd40b32 100644 --- a/core/src/retransmit_stage.rs +++ b/core/src/retransmit_stage.rs @@ -639,19 +639,19 @@ mod tests { assert!(should_skip_retransmit(&shred, &shreds_received)); assert!(should_skip_retransmit(&shred, &shreds_received)); - let shred = Shred::new_empty_coding(slot, index, 0, 1, 1, version); + let shred = Shred::new_empty_coding(slot, index, 0, 1, 1, 0, version); // Coding at (1, 5) passes assert!(!should_skip_retransmit(&shred, &shreds_received)); // then blocked assert!(should_skip_retransmit(&shred, &shreds_received)); - let shred = Shred::new_empty_coding(slot, index, 2, 1, 1, version); + let shred = Shred::new_empty_coding(slot, index, 2, 1, 1, 0, version); // 2nd unique coding at (1, 5) passes assert!(!should_skip_retransmit(&shred, &shreds_received)); // same again is blocked assert!(should_skip_retransmit(&shred, &shreds_received)); - let shred = Shred::new_empty_coding(slot, index, 3, 1, 1, version); + let shred = Shred::new_empty_coding(slot, index, 3, 1, 1, 0, version); // Another unique coding at (1, 5) always blocked assert!(should_skip_retransmit(&shred, &shreds_received)); assert!(should_skip_retransmit(&shred, &shreds_received)); diff --git a/core/src/window_service.rs b/core/src/window_service.rs index e56b333bd..919faaab1 100644 --- a/core/src/window_service.rs +++ b/core/src/window_service.rs @@ -878,7 +878,15 @@ mod test { )); // coding shreds don't contain parent slot information, test that slot >= root - let (common, coding) = Shredder::new_coding_shred_header(5, 5, 5, 6, 6, 0); + let (common, coding) = Shredder::new_coding_shred_header( + 5, // slot + 5, // index + 5, // fec_set_index + 6, // num_data_shreds + 6, // num_coding_shreds + 3, // position + 0, // version + ); let mut coding_shred = Shred::new_empty_from_header(common, DataShredHeader::default(), coding); Shredder::sign_shred(&leader_keypair, &mut coding_shred); @@ -954,7 +962,15 @@ mod test { std::net::{IpAddr, Ipv4Addr}, }; solana_logger::setup(); - let (common, coding) = Shredder::new_coding_shred_header(5, 5, 5, 6, 6, 0); + let (common, coding) = Shredder::new_coding_shred_header( + 5, // slot + 5, // index + 5, // fec_set_index + 6, // num_data_shreds + 6, // num_coding_shreds + 4, // position + 0, // version + ); let shred = Shred::new_empty_from_header(common, DataShredHeader::default(), coding); let mut shreds = vec![shred.clone(), shred.clone(), shred]; let _from_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 8080); diff --git a/ledger/src/blockstore.rs b/ledger/src/blockstore.rs index 3c82f70cf..8495024d2 100644 --- a/ledger/src/blockstore.rs +++ b/ledger/src/blockstore.rs @@ -5554,7 +5554,14 @@ pub mod tests { let blockstore = Blockstore::open(ledger_path.path()).unwrap(); let slot = 1; - let (shred, coding) = Shredder::new_coding_shred_header(slot, 11, 11, 11, 11, 0); + let (shred, coding) = Shredder::new_coding_shred_header( + slot, 11, // index + 11, // fec_set_index + 11, // num_data_shreds + 11, // num_coding_shreds + 8, // position + 0, // version + ); let coding_shred = Shred::new_empty_from_header(shred, DataShredHeader::default(), coding); let mut erasure_metas = HashMap::new(); @@ -5604,7 +5611,14 @@ pub mod tests { let last_root = RwLock::new(0); let slot = 1; - let (mut shred, coding) = Shredder::new_coding_shred_header(slot, 11, 11, 11, 11, 0); + let (mut shred, coding) = Shredder::new_coding_shred_header( + slot, 11, // index + 11, // fec_set_index + 11, // num_data_shreds + 11, // num_coding_shreds + 8, // position + 0, // version + ); let coding_shred = Shred::new_empty_from_header(shred.clone(), DataShredHeader::default(), coding.clone()); diff --git a/ledger/src/shred.rs b/ledger/src/shred.rs index 73da3c5da..48ab6efec 100644 --- a/ledger/src/shred.rs +++ b/ledger/src/shred.rs @@ -222,8 +222,7 @@ pub struct DataShredHeader { pub struct CodingShredHeader { pub num_data_shreds: u16, pub num_coding_shreds: u16, - #[serde(rename = "position")] - __unused: u16, + pub position: u16, } #[derive(Clone, Debug, PartialEq)] @@ -382,8 +381,9 @@ impl Shred { slot: Slot, index: u32, fec_set_index: u32, - num_data: usize, - num_code: usize, + num_data: u16, + num_code: u16, + position: u16, version: u16, ) -> Self { let (header, coding_header) = Shredder::new_coding_shred_header( @@ -392,6 +392,7 @@ impl Shred { fec_set_index, num_data, num_code, + position, version, ); Shred::new_empty_from_header(header, DataShredHeader::default(), coding_header) @@ -812,8 +813,9 @@ impl Shredder { slot: Slot, index: u32, fec_set_index: u32, - num_data: usize, - num_code: usize, + num_data_shreds: u16, + num_coding_shreds: u16, + position: u16, version: u16, ) -> (ShredCommonHeader, CodingShredHeader) { let header = ShredCommonHeader { @@ -827,9 +829,9 @@ impl Shredder { ( header, CodingShredHeader { - num_data_shreds: num_data as u16, - num_coding_shreds: num_code as u16, - ..CodingShredHeader::default() + num_data_shreds, + num_coding_shreds, + position, }, ) } @@ -865,6 +867,8 @@ impl Shredder { .unwrap() .encode(&data, &mut parity[..]) .unwrap(); + let num_data = u16::try_from(num_data).unwrap(); + let num_coding = u16::try_from(num_coding).unwrap(); parity .iter() .enumerate() @@ -875,6 +879,7 @@ impl Shredder { fec_set_index, num_data, num_coding, + u16::try_from(i).unwrap(), // position version, ); shred.payload[SIZE_OF_CODING_SHRED_HEADERS..].copy_from_slice(parity); @@ -1882,7 +1887,15 @@ pub mod tests { ); assert_eq!(stats.index_overrun, 4); - let shred = Shred::new_empty_coding(8, 2, 10, 30, 4, 200); + let shred = Shred::new_empty_coding( + 8, // slot + 2, // index + 10, // fec_set_index + 30, // num_data + 4, // num_code + 1, // position + 200, // version + ); shred.copy_to_packet(&mut packet); assert_eq!( Some((8, 2, ShredType::Code)), @@ -1894,7 +1907,15 @@ pub mod tests { assert_eq!(None, get_shred_slot_index_type(&packet, &mut stats)); assert_eq!(1, stats.index_out_of_bounds); - let (header, coding_header) = Shredder::new_coding_shred_header(8, 2, 10, 30, 4, 200); + let (header, coding_header) = Shredder::new_coding_shred_header( + 8, // slot + 2, // index + 10, // fec_set_index + 30, // num_data_shreds + 4, // num_coding_shreds + 3, // position + 200, // version + ); let shred = Shred::new_empty_from_header(header, DataShredHeader::default(), coding_header); shred.copy_to_packet(&mut packet); packet.data[OFFSET_OF_SHRED_TYPE] = u8::MAX;