From d94041e98de34914d18e0ee65c70d336579849b0 Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Thu, 12 Dec 2019 16:50:29 -0800 Subject: [PATCH] Allow coding shred index to be different than data shred index (#7438) * Allow coding shred index to be different than data shred index * move fec_set_index to shred's common header * fix bench --- core/benches/shredder.rs | 2 +- .../broadcast_stage/standard_broadcast_run.rs | 1 + core/src/chacha.rs | 2 +- core/src/sigverify_shreds.rs | 28 ++++++- core/src/window_service.rs | 2 +- ledger/src/blocktree.rs | 40 +++++++--- ledger/src/blocktree_meta.rs | 35 +++------ ledger/src/shred.rs | 75 +++++++++++++++++-- ledger/src/sigverify_shreds.rs | 53 +++++++++++-- ledger/tests/shred.rs | 1 + 10 files changed, 186 insertions(+), 53 deletions(-) diff --git a/core/benches/shredder.rs b/core/benches/shredder.rs index 4efaeeaef6ade5..4bfc009a6986ae 100644 --- a/core/benches/shredder.rs +++ b/core/benches/shredder.rs @@ -75,7 +75,7 @@ fn bench_deshredder(bencher: &mut Bencher) { fn bench_deserialize_hdr(bencher: &mut Bencher) { let data = vec![0; SIZE_OF_DATA_SHRED_PAYLOAD]; - let shred = Shred::new_from_data(2, 1, 1, Some(&data), true, true, 0, 0); + let shred = Shred::new_from_data(2, 1, 1, Some(&data), true, true, 0, 0, 1); bencher.iter(|| { let payload = shred.payload.clone(); diff --git a/core/src/broadcast_stage/standard_broadcast_run.rs b/core/src/broadcast_stage/standard_broadcast_run.rs index 36d48c23ef0baa..ac8d59bf47d852 100644 --- a/core/src/broadcast_stage/standard_broadcast_run.rs +++ b/core/src/broadcast_stage/standard_broadcast_run.rs @@ -66,6 +66,7 @@ impl StandardBroadcastRun { true, max_ticks_in_slot & SHRED_TICK_REFERENCE_MASK, self.shred_version, + last_unfinished_slot.next_shred_index, )) } else { None diff --git a/core/src/chacha.rs b/core/src/chacha.rs index fbeb70413b37a2..b4f799cd10d0ec 100644 --- a/core/src/chacha.rs +++ b/core/src/chacha.rs @@ -166,7 +166,7 @@ mod tests { hasher.hash(&buf[..size]); // golden needs to be updated if shred structure changes.... - let golden: Hash = "9K6NR4cazo7Jzk2CpyXmNaZMGqvfXG83JzyJipkoHare" + let golden: Hash = "2rq8nR6rns2T5zsbQAGBDZb41NVtacneLgkCH17CVxZm" .parse() .unwrap(); diff --git a/core/src/sigverify_shreds.rs b/core/src/sigverify_shreds.rs index 60e63370f52f1f..55d70b1fbcea0f 100644 --- a/core/src/sigverify_shreds.rs +++ b/core/src/sigverify_shreds.rs @@ -92,6 +92,7 @@ pub mod tests { true, 0, 0, + 0xc0de, ); let mut batch = [Packets::default(), Packets::default()]; @@ -110,6 +111,7 @@ pub mod tests { true, 0, 0, + 0xc0de, ); Shredder::sign_shred(&keypair, &mut shred); batch[1].packets.resize(1, Packet::default()); @@ -133,14 +135,32 @@ pub mod tests { let mut batch = vec![Packets::default()]; batch[0].packets.resize(2, Packet::default()); - let mut shred = - Shred::new_from_data(0, 0xc0de, 0xdead, Some(&[1, 2, 3, 4]), true, true, 0, 0); + let mut shred = Shred::new_from_data( + 0, + 0xc0de, + 0xdead, + Some(&[1, 2, 3, 4]), + true, + true, + 0, + 0, + 0xc0de, + ); Shredder::sign_shred(&leader_keypair, &mut shred); batch[0].packets[0].data[0..shred.payload.len()].copy_from_slice(&shred.payload); batch[0].packets[0].meta.size = shred.payload.len(); - let mut shred = - Shred::new_from_data(0, 0xbeef, 0xc0de, Some(&[1, 2, 3, 4]), true, true, 0, 0); + let mut shred = Shred::new_from_data( + 0, + 0xbeef, + 0xc0de, + Some(&[1, 2, 3, 4]), + true, + true, + 0, + 0, + 0xc0de, + ); let wrong_keypair = Keypair::new(); Shredder::sign_shred(&wrong_keypair, &mut shred); batch[0].packets[1].data[0..shred.payload.len()].copy_from_slice(&shred.payload); diff --git a/core/src/window_service.rs b/core/src/window_service.rs index 90818633e4b601..f9d6bb14c26092 100644 --- a/core/src/window_service.rs +++ b/core/src/window_service.rs @@ -363,7 +363,7 @@ mod test { ); // If it's a coding shred, test that slot >= root - let (common, coding) = Shredder::new_coding_shred_header(5, 5, 6, 6, 0, 0); + let (common, coding) = Shredder::new_coding_shred_header(5, 5, 5, 6, 6, 0, 0); let mut coding_shred = Shred::new_empty_from_header(common, DataShredHeader::default(), coding); Shredder::sign_shred(&leader_keypair, &mut coding_shred); diff --git a/ledger/src/blocktree.rs b/ledger/src/blocktree.rs index 60dc008582ca2a..3527e7640733fd 100644 --- a/ledger/src/blocktree.rs +++ b/ledger/src/blocktree.rs @@ -391,7 +391,11 @@ impl Blocktree { "blocktree-erasure", ("slot", slot as i64, i64), ("start_index", set_index as i64, i64), - ("end_index", erasure_meta.end_indexes().0 as i64, i64), + ( + "end_index", + (erasure_meta.set_index + erasure_meta.config.num_data() as u64) as i64, + i64 + ), ("recovery_attempted", attempted, bool), ("recovery_status", status, String), ("recovered", recovered as i64, i64), @@ -424,8 +428,10 @@ impl Blocktree { } } }); - (set_index..set_index + erasure_meta.config.num_coding() as u64).for_each( - |i| { + (erasure_meta.first_coding_index + ..erasure_meta.first_coding_index + + erasure_meta.config.num_coding() as u64) + .for_each(|i| { if let Some(shred) = prev_inserted_codes .remove(&(slot, i)) .map(|s| { @@ -454,13 +460,13 @@ impl Blocktree { { available_shreds.push(shred); } - }, - ); + }); if let Ok(mut result) = Shredder::try_recovery( available_shreds, erasure_meta.config.num_data(), erasure_meta.config.num_coding(), set_index as usize, + erasure_meta.first_coding_index as usize, slot, ) { submit_metrics(true, "complete".into(), result.len()); @@ -687,17 +693,21 @@ impl Blocktree { if is_trusted || Blocktree::should_insert_coding_shred(&shred, index_meta.coding(), &self.last_root) { - let set_index = shred_index - u64::from(shred.coding_header.position); + let set_index = u64::from(shred.common_header.fec_set_index); let erasure_config = ErasureConfig::new( shred.coding_header.num_data_shreds as usize, shred.coding_header.num_coding_shreds as usize, ); let erasure_meta = erasure_metas.entry((slot, set_index)).or_insert_with(|| { + let first_coding_index = + u64::from(shred.index()) - u64::from(shred.coding_header.position); self.erasure_meta_cf .get((slot, set_index)) .expect("Expect database get to succeed") - .unwrap_or_else(|| ErasureMeta::new(set_index, &erasure_config)) + .unwrap_or_else(|| { + ErasureMeta::new(set_index, first_coding_index, &erasure_config) + }) }); if erasure_config != erasure_meta.config { @@ -3535,7 +3545,17 @@ pub mod tests { let gap: u64 = 10; let shreds: Vec<_> = (0..64) .map(|i| { - Shred::new_from_data(slot, (i * gap) as u32, 0, None, false, false, i as u8, 0) + Shred::new_from_data( + slot, + (i * gap) as u32, + 0, + None, + false, + false, + i as u8, + 0, + (i * gap) as u32, + ) }) .collect(); blocktree.insert_shreds(shreds, None, false).unwrap(); @@ -3726,7 +3746,8 @@ 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, 10, 0); + let (mut shred, coding) = + Shredder::new_coding_shred_header(slot, 11, 11, 11, 11, 10, 0); let coding_shred = Shred::new_empty_from_header( shred.clone(), DataShredHeader::default(), @@ -4149,6 +4170,7 @@ pub mod tests { true, 0, 0, + next_shred_index as u32, )]; // With the corruption, nothing should be returned, even though an diff --git a/ledger/src/blocktree_meta.rs b/ledger/src/blocktree_meta.rs index 5e5e9734b3d418..4d195de8379f3a 100644 --- a/ledger/src/blocktree_meta.rs +++ b/ledger/src/blocktree_meta.rs @@ -59,6 +59,8 @@ pub struct CodingIndex { pub struct ErasureMeta { /// Which erasure set in the slot this is pub set_index: u64, + /// First coding index in the FEC set + pub first_coding_index: u64, /// Size of shards in this erasure set pub size: usize, /// Erasure configuration for this erasure set @@ -200,9 +202,10 @@ impl SlotMeta { } impl ErasureMeta { - pub fn new(set_index: u64, config: &ErasureConfig) -> ErasureMeta { + pub fn new(set_index: u64, first_coding_index: u64, config: &ErasureConfig) -> ErasureMeta { ErasureMeta { set_index, + first_coding_index, size: 0, config: *config, } @@ -211,11 +214,12 @@ impl ErasureMeta { pub fn status(&self, index: &Index) -> ErasureMetaStatus { use ErasureMetaStatus::*; - let start_idx = self.start_index(); - let (data_end_idx, coding_end_idx) = self.end_indexes(); - - let num_coding = index.coding().present_in_bounds(start_idx..coding_end_idx); - let num_data = index.data().present_in_bounds(start_idx..data_end_idx); + let num_coding = index.coding().present_in_bounds( + self.first_coding_index..self.first_coding_index + self.config.num_coding() as u64, + ); + let num_data = index + .data() + .present_in_bounds(self.set_index..self.set_index + self.config.num_data() as u64); let (data_missing, coding_missing) = ( self.config.num_data() - num_data, @@ -240,23 +244,6 @@ impl ErasureMeta { pub fn size(&self) -> usize { self.size } - - pub fn set_index_for(index: u64, num_data: usize) -> u64 { - index / num_data as u64 - } - - pub fn start_index(&self) -> u64 { - self.set_index - } - - /// returns a tuple of (data_end, coding_end) - pub fn end_indexes(&self) -> (u64, u64) { - let start = self.start_index(); - ( - start + self.config.num_data() as u64, - start + self.config.num_coding() as u64, - ) - } } #[cfg(test)] @@ -272,7 +259,7 @@ mod test { let set_index = 0; let erasure_config = ErasureConfig::default(); - let mut e_meta = ErasureMeta::new(set_index, &erasure_config); + let mut e_meta = ErasureMeta::new(set_index, set_index, &erasure_config); let mut rng = thread_rng(); let mut index = Index::new(0); e_meta.size = 1; diff --git a/ledger/src/shred.rs b/ledger/src/shred.rs index b66e6d54784d53..43656194ea3bfb 100644 --- a/ledger/src/shred.rs +++ b/ledger/src/shred.rs @@ -27,7 +27,7 @@ use thiserror::Error; /// The following constants are computed by hand, and hardcoded. /// `test_shred_constants` ensures that the values are correct. /// Constants are used over lazy_static for performance reasons. -pub const SIZE_OF_COMMON_SHRED_HEADER: usize = 79; +pub const SIZE_OF_COMMON_SHRED_HEADER: usize = 83; pub const SIZE_OF_DATA_SHRED_HEADER: usize = 3; pub const SIZE_OF_CODING_SHRED_HEADER: usize = 6; pub const SIZE_OF_SIGNATURE: usize = 64; @@ -87,6 +87,7 @@ pub struct ShredCommonHeader { pub slot: Slot, pub index: u32, pub version: u16, + pub fec_set_index: u32, } /// The data shred header has parent offset and flags @@ -153,12 +154,14 @@ impl Shred { is_last_in_slot: bool, reference_tick: u8, version: u16, + fec_set_index: u32, ) -> Self { let mut payload = vec![0; PACKET_DATA_SIZE]; let common_header = ShredCommonHeader { slot, index, version, + fec_set_index, ..ShredCommonHeader::default() }; @@ -460,6 +463,11 @@ impl Shredder { .map(|(i, shred_data)| { let shred_index = next_shred_index + i as u32; + // Each FEC block has maximum MAX_DATA_SHREDS_PER_FEC_BLOCK shreds + // "FEC set index" is the index of first data shred in that FEC block + let fec_set_index = + shred_index - (i % MAX_DATA_SHREDS_PER_FEC_BLOCK as usize) as u32; + let (is_last_data, is_last_in_slot) = { if shred_index == last_shred_index { (true, is_last_in_slot) @@ -477,6 +485,7 @@ impl Shredder { is_last_in_slot, self.reference_tick, self.version, + fec_set_index, ); Shredder::sign_shred(&self.keypair, &mut shred); @@ -541,6 +550,7 @@ impl Shredder { pub fn new_coding_shred_header( slot: Slot, index: u32, + fec_set_index: u32, num_data: usize, num_code: usize, position: usize, @@ -551,6 +561,7 @@ impl Shredder { index, slot, version, + fec_set_index, ..ShredCommonHeader::default() }; ( @@ -592,6 +603,7 @@ impl Shredder { let (header, coding_header) = Self::new_coding_shred_header( slot, start_index + i as u32, + start_index, num_data, num_coding, i, @@ -622,6 +634,7 @@ impl Shredder { let (common_header, coding_header) = Self::new_coding_shred_header( slot, start_index + i as u32, + start_index, num_data, num_coding, i, @@ -679,6 +692,7 @@ impl Shredder { num_data: usize, num_coding: usize, first_index: usize, + first_code_index: usize, slot: Slot, ) -> std::result::Result, reed_solomon_erasure::Error> { let mut recovered_data = vec![]; @@ -691,7 +705,8 @@ impl Shredder { let mut shred_bufs: Vec> = shreds .into_iter() .flat_map(|shred| { - let index = Self::get_shred_index(&shred, num_data); + let index = + Self::get_shred_index(&shred, num_data, first_index, first_code_index); let mut blocks = Self::fill_in_missing_shreds( num_data, num_coding, @@ -789,11 +804,16 @@ impl Shredder { Ok(Self::reassemble_payload(num_data, data_shred_bufs)) } - fn get_shred_index(shred: &Shred, num_data: usize) -> usize { + fn get_shred_index( + shred: &Shred, + num_data: usize, + first_data_index: usize, + first_code_index: usize, + ) -> usize { if shred.is_data() { shred.index() as usize } else { - shred.index() as usize + num_data + shred.index() as usize + num_data + first_data_index - first_code_index } } @@ -1168,6 +1188,7 @@ pub mod tests { num_data_shreds, num_data_shreds, 0, + 0, slot ), Err(reed_solomon_erasure::Error::TooFewShardsPresent) @@ -1179,6 +1200,7 @@ pub mod tests { num_data_shreds, num_data_shreds, 0, + 0, slot, ) .unwrap(); @@ -1196,6 +1218,7 @@ pub mod tests { num_data_shreds, num_data_shreds, 0, + 0, slot, ) .unwrap(); @@ -1243,6 +1266,7 @@ pub mod tests { num_data_shreds, num_data_shreds, 0, + 0, slot, ) .unwrap(); @@ -1315,6 +1339,7 @@ pub mod tests { num_data_shreds, num_data_shreds, 25, + 25, slot, ) .unwrap(); @@ -1346,6 +1371,7 @@ pub mod tests { num_data_shreds, num_data_shreds, 25, + 25, slot + 1, ) .unwrap(); @@ -1358,6 +1384,7 @@ pub mod tests { num_data_shreds, num_data_shreds, 15, + 15, slot, ), Err(reed_solomon_erasure::Error::TooFewShardsPresent) @@ -1365,7 +1392,7 @@ pub mod tests { // Test8: Try recovery/reassembly with incorrect index. Hint: does not recover any shreds assert_matches!( - Shredder::try_recovery(shred_info, num_data_shreds, num_data_shreds, 35, slot,), + Shredder::try_recovery(shred_info, num_data_shreds, num_data_shreds, 35, 35, slot,), Err(reed_solomon_erasure::Error::TooFewShardsPresent) ); } @@ -1419,4 +1446,42 @@ pub mod tests { let version = Shred::version_from_hash(&Hash::new(&hash)); assert_eq!(version, 0x5a5a); } + + #[test] + fn test_shred_fec_set_index() { + let keypair = Arc::new(Keypair::new()); + let hash = hash(Hash::default().as_ref()); + let version = Shred::version_from_hash(&hash); + assert_ne!(version, 0); + let shredder = + Shredder::new(0, 0, 0.5, keypair, 0, version).expect("Failed in creating shredder"); + + let entries: Vec<_> = (0..500) + .map(|_| { + let keypair0 = Keypair::new(); + let keypair1 = Keypair::new(); + let tx0 = + system_transaction::transfer(&keypair0, &keypair1.pubkey(), 1, Hash::default()); + Entry::new(&Hash::default(), 1, vec![tx0]) + }) + .collect(); + + let start_index = 0x12; + let (data_shreds, coding_shreds, _next_index) = + shredder.entries_to_shreds(&entries, true, start_index); + + let max_per_block = MAX_DATA_SHREDS_PER_FEC_BLOCK as usize; + data_shreds.iter().enumerate().for_each(|(i, s)| { + let expected_fec_set_index = start_index + ((i / max_per_block) * max_per_block) as u32; + assert_eq!(s.common_header.fec_set_index, expected_fec_set_index); + }); + + coding_shreds.iter().enumerate().for_each(|(i, s)| { + // There'll be half the number of coding shreds, as FEC rate is 0.5 + // So multiply i with 2 + let expected_fec_set_index = + start_index + ((i * 2 / max_per_block) * max_per_block) as u32; + assert_eq!(s.common_header.fec_set_index, expected_fec_set_index); + }); + } } diff --git a/ledger/src/sigverify_shreds.rs b/ledger/src/sigverify_shreds.rs index 1310caad7de859..a75fcb65c1ec50 100644 --- a/ledger/src/sigverify_shreds.rs +++ b/ledger/src/sigverify_shreds.rs @@ -456,8 +456,17 @@ pub mod tests { solana_logger::setup(); let mut packet = Packet::default(); let slot = 0xdeadc0de; - let mut shred = - Shred::new_from_data(slot, 0xc0de, 0xdead, Some(&[1, 2, 3, 4]), true, true, 0, 0); + let mut shred = Shred::new_from_data( + slot, + 0xc0de, + 0xdead, + Some(&[1, 2, 3, 4]), + true, + true, + 0, + 0, + 0xc0de, + ); assert_eq!(shred.slot(), slot); let keypair = Keypair::new(); Shredder::sign_shred(&keypair, &mut shred); @@ -490,8 +499,17 @@ pub mod tests { solana_logger::setup(); let mut batch = [Packets::default()]; let slot = 0xdeadc0de; - let mut shred = - Shred::new_from_data(slot, 0xc0de, 0xdead, Some(&[1, 2, 3, 4]), true, true, 0, 0); + let mut shred = Shred::new_from_data( + slot, + 0xc0de, + 0xdead, + Some(&[1, 2, 3, 4]), + true, + true, + 0, + 0, + 0xc0de, + ); let keypair = Keypair::new(); Shredder::sign_shred(&keypair, &mut shred); batch[0].packets.resize(1, Packet::default()); @@ -533,8 +551,17 @@ pub mod tests { let mut batch = [Packets::default()]; let slot = 0xdeadc0de; - let mut shred = - Shred::new_from_data(slot, 0xc0de, 0xdead, Some(&[1, 2, 3, 4]), true, true, 0, 0); + let mut shred = Shred::new_from_data( + slot, + 0xc0de, + 0xdead, + Some(&[1, 2, 3, 4]), + true, + true, + 0, + 0, + 0xc0de, + ); let keypair = Keypair::new(); Shredder::sign_shred(&keypair, &mut shred); batch[0].packets.resize(1, Packet::default()); @@ -598,6 +625,7 @@ pub mod tests { true, 1, 2, + 0xc0de, ); shred.copy_to_packet(p); } @@ -631,8 +659,17 @@ pub mod tests { let mut batch = [Packets::default()]; let slot = 0xdeadc0de; let keypair = Keypair::new(); - let shred = - Shred::new_from_data(slot, 0xc0de, 0xdead, Some(&[1, 2, 3, 4]), true, true, 0, 0); + let shred = Shred::new_from_data( + slot, + 0xc0de, + 0xdead, + Some(&[1, 2, 3, 4]), + true, + true, + 0, + 0, + 0xc0de, + ); batch[0].packets.resize(1, Packet::default()); batch[0].packets[0].data[0..shred.payload.len()].copy_from_slice(&shred.payload); batch[0].packets[0].meta.size = shred.payload.len(); diff --git a/ledger/tests/shred.rs b/ledger/tests/shred.rs index 461fb8483cd01e..05d6b27036341b 100644 --- a/ledger/tests/shred.rs +++ b/ledger/tests/shred.rs @@ -63,6 +63,7 @@ fn test_multi_fec_block_coding() { MAX_DATA_SHREDS_PER_FEC_BLOCK as usize, MAX_DATA_SHREDS_PER_FEC_BLOCK as usize, shred_start_index, + shred_start_index, slot, ) .unwrap();