From 37296caee829faa399cd5034df007867593cd9c0 Mon Sep 17 00:00:00 2001 From: behzad nouri Date: Fri, 9 Sep 2022 17:58:04 +0000 Subject: [PATCH] removes set_{slot,index,last_in_slot} implementations for merkle shreds (#27689) These methods are only used in tests but invoked on a merkle shred they will always invalidate the shred because the merkle proof will no longer verify. As a result the shred will not sanitize and blockstore will avoid inserting them. Their use in tests will result in spurious test coverage because the shreds will not be ingested. The commit removes implementation of these methods for merkle shreds. Follow up commits will entirely remove these methods from shreds api. --- ledger/src/shred/common.rs | 22 ++++++++++++++++++---- ledger/src/shred/legacy.rs | 14 +++++++------- ledger/src/shred/merkle.rs | 13 +++---------- ledger/src/shred/shred_data.rs | 9 ++++++++- ledger/src/shred/traits.rs | 3 --- 5 files changed, 36 insertions(+), 25 deletions(-) diff --git a/ledger/src/shred/common.rs b/ledger/src/shred/common.rs index 3478001a0c..330e4a8da9 100644 --- a/ledger/src/shred/common.rs +++ b/ledger/src/shred/common.rs @@ -51,14 +51,28 @@ macro_rules! impl_shred_common { // Only for tests. fn set_index(&mut self, index: u32) { - self.common_header.index = index; - bincode::serialize_into(&mut self.payload[..], &self.common_header).unwrap(); + match self.common_header.shred_variant { + ShredVariant::LegacyCode | ShredVariant::LegacyData => { + self.common_header.index = index; + bincode::serialize_into(&mut self.payload[..], &self.common_header).unwrap(); + } + ShredVariant::MerkleCode(_) | ShredVariant::MerkleData(_) => { + panic!("Not Implemented!"); + } + } } // Only for tests. fn set_slot(&mut self, slot: Slot) { - self.common_header.slot = slot; - bincode::serialize_into(&mut self.payload[..], &self.common_header).unwrap(); + match self.common_header.shred_variant { + ShredVariant::LegacyCode | ShredVariant::LegacyData => { + self.common_header.slot = slot; + bincode::serialize_into(&mut self.payload[..], &self.common_header).unwrap(); + } + ShredVariant::MerkleCode(_) | ShredVariant::MerkleData(_) => { + panic!("Not Implemented!"); + } + } } }; } diff --git a/ledger/src/shred/legacy.rs b/ledger/src/shred/legacy.rs index 1096ef1f54..62eb58bc38 100644 --- a/ledger/src/shred/legacy.rs +++ b/ledger/src/shred/legacy.rs @@ -196,13 +196,6 @@ impl ShredDataTrait for ShredData { } Ok(&self.payload[Self::SIZE_OF_HEADERS..size]) } - - // Only for tests. - fn set_last_in_slot(&mut self) { - self.data_header.flags |= ShredFlags::LAST_SHRED_IN_SLOT; - let buffer = &mut self.payload[SIZE_OF_COMMON_SHRED_HEADER..]; - bincode::serialize_into(buffer, &self.data_header).unwrap(); - } } impl ShredCodeTrait for ShredCode { @@ -278,6 +271,13 @@ impl ShredData { shred.resize(Self::SIZE_OF_PAYLOAD, 0u8); Ok(shred) } + + // Only for tests. + pub(crate) fn set_last_in_slot(&mut self) { + self.data_header.flags |= ShredFlags::LAST_SHRED_IN_SLOT; + let buffer = &mut self.payload[SIZE_OF_COMMON_SHRED_HEADER..]; + bincode::serialize_into(buffer, &self.data_header).unwrap(); + } } impl ShredCode { diff --git a/ledger/src/shred/merkle.rs b/ledger/src/shred/merkle.rs index 9d0482b953..147955d3a9 100644 --- a/ledger/src/shred/merkle.rs +++ b/ledger/src/shred/merkle.rs @@ -8,9 +8,8 @@ use { traits::{ Shred as ShredTrait, ShredCode as ShredCodeTrait, ShredData as ShredDataTrait, }, - CodingShredHeader, DataShredHeader, Error, ShredCommonHeader, ShredFlags, ShredVariant, - SIZE_OF_CODING_SHRED_HEADERS, SIZE_OF_COMMON_SHRED_HEADER, SIZE_OF_DATA_SHRED_HEADERS, - SIZE_OF_SIGNATURE, + CodingShredHeader, DataShredHeader, Error, ShredCommonHeader, ShredVariant, + SIZE_OF_CODING_SHRED_HEADERS, SIZE_OF_DATA_SHRED_HEADERS, SIZE_OF_SIGNATURE, }, shredder::ReedSolomon, }, @@ -511,13 +510,6 @@ impl ShredDataTrait for ShredData { } Ok(&self.payload[Self::SIZE_OF_HEADERS..size]) } - - // Only for tests. - fn set_last_in_slot(&mut self) { - self.data_header.flags |= ShredFlags::LAST_SHRED_IN_SLOT; - let buffer = &mut self.payload[SIZE_OF_COMMON_SHRED_HEADER..]; - bincode::serialize_into(buffer, &self.data_header).unwrap(); - } } impl ShredCodeTrait for ShredCode { @@ -746,6 +738,7 @@ pub(super) fn recover(mut shreds: Vec) -> Result, Error> { mod test { use { super::*, + crate::shred::ShredFlags, itertools::Itertools, matches::assert_matches, rand::{seq::SliceRandom, CryptoRng, Rng}, diff --git a/ledger/src/shred/shred_data.rs b/ledger/src/shred/shred_data.rs index d6976514e2..73c8ef2216 100644 --- a/ledger/src/shred/shred_data.rs +++ b/ledger/src/shred/shred_data.rs @@ -28,7 +28,6 @@ impl ShredData { dispatch!(pub(super) fn parent(&self) -> Result); dispatch!(pub(super) fn payload(&self) -> &Vec); dispatch!(pub(super) fn sanitize(&self) -> Result<(), Error>); - dispatch!(pub(super) fn set_last_in_slot(&mut self)); dispatch!(pub(super) fn set_signature(&mut self, signature: Signature)); dispatch!(pub(super) fn signed_message(&self) -> &[u8]); @@ -105,6 +104,14 @@ impl ShredData { Some(proof_size) => merkle::ShredData::capacity(proof_size), } } + + // Only for tests. + pub(super) fn set_last_in_slot(&mut self) { + match self { + Self::Legacy(shred) => shred.set_last_in_slot(), + Self::Merkle(_) => panic!("Not Implemented!"), + } + } } impl From for ShredData { diff --git a/ledger/src/shred/traits.rs b/ledger/src/shred/traits.rs index b5326b62a7..5ae2b955aa 100644 --- a/ledger/src/shred/traits.rs +++ b/ledger/src/shred/traits.rs @@ -54,9 +54,6 @@ pub(super) trait ShredData: Shred { } fn data(&self) -> Result<&[u8], Error>; - - // Only for tests. - fn set_last_in_slot(&mut self); } pub(super) trait ShredCode: Shred {