From 6d1e1287bcf14eb2c871c99cf297ff7e5deed637 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Sun, 5 Dec 2021 17:12:12 +0000 Subject: [PATCH] adds back position field to coding-shred-header (backport #21600) (#21619) * 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. (cherry picked from commit cd17f63d818fc5f403678f670a9384579a5cfb9c) # Conflicts: # core/src/window_service.rs # ledger/src/blockstore.rs # ledger/src/shred.rs * removes backport merge conflicts Co-authored-by: behzad nouri --- core/src/retransmit_stage.rs | 6 ++--- core/src/window_service.rs | 20 +++++++++++++-- ledger/src/blockstore.rs | 18 +++++++++++-- ledger/src/shred.rs | 50 +++++++++++++++++++++++++++--------- 4 files changed, 75 insertions(+), 19 deletions(-) diff --git a/core/src/retransmit_stage.rs b/core/src/retransmit_stage.rs index a602c9362d..256b2c1c25 100644 --- a/core/src/retransmit_stage.rs +++ b/core/src/retransmit_stage.rs @@ -632,19 +632,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 3438b16178..2e8344cfe8 100644 --- a/core/src/window_service.rs +++ b/core/src/window_service.rs @@ -791,7 +791,15 @@ mod test { )); // If it's a coding shred, 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); @@ -918,7 +926,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 f0f0b44db8..96a83c65fe 100644 --- a/ledger/src/blockstore.rs +++ b/ledger/src/blockstore.rs @@ -5717,7 +5717,14 @@ pub mod tests { let blockstore = Blockstore::open(&blockstore_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); @@ -5767,7 +5774,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(), diff --git a/ledger/src/shred.rs b/ledger/src/shred.rs index f922a18631..d6a6acb4b5 100644 --- a/ledger/src/shred.rs +++ b/ledger/src/shred.rs @@ -75,7 +75,12 @@ use { pubkey::Pubkey, signature::{Keypair, Signature, Signer}, }, - std::{convert::TryInto, mem::size_of, ops::Deref, sync::Arc}, + std::{ + convert::{TryFrom, TryInto}, + mem::size_of, + ops::Deref, + sync::Arc, + }, thiserror::Error, }; @@ -203,8 +208,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)] @@ -362,8 +366,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( @@ -372,6 +377,7 @@ impl Shred { fec_set_index, num_data, num_code, + position, version, ); Shred::new_empty_from_header(header, DataShredHeader::default(), coding_header) @@ -757,8 +763,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 { @@ -772,9 +779,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, }, ) } @@ -810,6 +817,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() @@ -820,6 +829,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); @@ -1947,7 +1957,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, false)), @@ -1959,7 +1977,15 @@ pub mod tests { assert_eq!(None, get_shred_slot_index_type(&packet, &mut stats)); assert_eq!(1, stats.index_out_of_bounds); - let (mut header, coding_header) = Shredder::new_coding_shred_header(8, 2, 10, 30, 4, 200); + let (mut 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 + ); header.shred_type = ShredType(u8::MAX); let shred = Shred::new_empty_from_header(header, DataShredHeader::default(), coding_header); shred.copy_to_packet(&mut packet);