Skip to content

Commit

Permalink
v2.0: gossip: ignore retransmitter signatures when comparing duplicat…
Browse files Browse the repository at this point in the history
…e shreds (backport of #2673) (#2699)

* gossip: ignore retransmitter signatures when comparing duplicate shreds (#2673)

* gossip: ignore retransmitter signatures when comparing duplicate shreds

* pr feedback: compare rest of payload instead of setting sig

* pr feedback: remove dcou, pub(super)

(cherry picked from commit ff87ed9)

# Conflicts:
#	ledger/src/shred.rs

* fix conflicts

---------

Co-authored-by: Ashwin Sekar <[email protected]>
Co-authored-by: Ashwin Sekar <[email protected]>
  • Loading branch information
3 people authored Aug 23, 2024
1 parent 901de9a commit 590a23c
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 5 deletions.
85 changes: 82 additions & 3 deletions gossip/src/duplicate_shred.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,8 @@ pub enum Error {
/// - Must match the expected shred version
/// - Must both sigverify for the correct leader
/// - Must have a merkle root conflict, otherwise `shred1` and `shred2` must have the same `shred_type`
/// - If `shred1` and `shred2` share the same index they must be not equal
/// - If `shred1` and `shred2` share the same index they must be not have equal payloads excluding the
/// retransmitter signature
/// - If `shred1` and `shred2` do not share the same index and are data shreds
/// verify that they indicate an index conflict. One of them must be the
/// LAST_SHRED_IN_SLOT, however the other shred must have a higher index.
Expand Down Expand Up @@ -147,7 +148,7 @@ where
}

if shred1.index() == shred2.index() {
if shred1.payload() != shred2.payload() {
if shred1.is_shred_duplicate(shred2) {
return Ok(());
}
return Err(Error::InvalidDuplicateShreds);
Expand Down Expand Up @@ -314,7 +315,7 @@ pub(crate) mod tests {
solana_ledger::shred::{ProcessShredsStats, ReedSolomonCache, Shredder},
solana_sdk::{
hash::Hash,
signature::{Keypair, Signer},
signature::{Keypair, Signature, Signer},
system_transaction,
},
std::sync::Arc,
Expand Down Expand Up @@ -1255,4 +1256,82 @@ pub(crate) mod tests {
);
}
}

#[test]
fn test_retransmitter_signature_invalid() {
let mut rng = rand::thread_rng();
let leader = Arc::new(Keypair::new());
let (slot, parent_slot, reference_tick, version) = (53084024, 53084023, 0, 0);
let shredder = Shredder::new(slot, parent_slot, reference_tick, version).unwrap();
let next_shred_index = rng.gen_range(0..32_000);
let leader_schedule = |s| {
if s == slot {
Some(leader.pubkey())
} else {
None
}
};
let data_shred =
new_rand_data_shred(&mut rng, next_shred_index, &shredder, &leader, true, true);
let coding_shred =
new_rand_coding_shreds(&mut rng, next_shred_index, 10, &shredder, &leader, true)[0]
.clone();
let mut data_shred_different_retransmitter_payload = data_shred.clone().into_payload();
shred::layout::set_retransmitter_signature(
&mut data_shred_different_retransmitter_payload,
&Signature::new_unique(),
)
.unwrap();
let data_shred_different_retransmitter =
Shred::new_from_serialized_shred(data_shred_different_retransmitter_payload).unwrap();
let mut coding_shred_different_retransmitter_payload = coding_shred.clone().into_payload();
shred::layout::set_retransmitter_signature(
&mut coding_shred_different_retransmitter_payload,
&Signature::new_unique(),
)
.unwrap();
let coding_shred_different_retransmitter =
Shred::new_from_serialized_shred(coding_shred_different_retransmitter_payload).unwrap();

let test_cases = vec![
// Same data shred from different retransmitter
(data_shred, data_shred_different_retransmitter),
// Same coding shred from different retransmitter
(coding_shred, coding_shred_different_retransmitter),
];
for (shred1, shred2) in test_cases.iter().flat_map(|(a, b)| [(a, b), (b, a)]) {
assert_matches!(
from_shred(
shred1.clone(),
Pubkey::new_unique(), // self_pubkey
shred2.payload().clone(),
Some(leader_schedule),
rng.gen(), // wallclock
512, // max_size
version,
)
.err()
.unwrap(),
Error::InvalidDuplicateShreds
);

let chunks: Vec<_> = from_shred_bypass_checks(
shred1.clone(),
Pubkey::new_unique(), // self_pubkey
shred2.clone(),
rng.gen(), // wallclock
512, // max_size
)
.unwrap()
.collect();
assert!(chunks.len() > 4);

assert_matches!(
into_shreds(&leader.pubkey(), chunks, version)
.err()
.unwrap(),
Error::InvalidDuplicateShreds
);
}
}
}
33 changes: 32 additions & 1 deletion ledger/src/shred.rs
Original file line number Diff line number Diff line change
Expand Up @@ -576,6 +576,37 @@ impl Shred {
Self::ShredData(_) => Err(Error::InvalidShredType),
}
}

/// Returns true if the other shred has the same ShredId, i.e. (slot, index,
/// shred-type), but different payload.
/// Retransmitter's signature is ignored when comparing payloads.
pub fn is_shred_duplicate(&self, other: &Shred) -> bool {
if self.id() != other.id() {
return false;
}
fn get_payload(shred: &Shred) -> &[u8] {
let Ok(offset) = shred.retransmitter_signature_offset() else {
return shred.payload();
};
// Assert that the retransmitter's signature is at the very end of
// the shred payload.
debug_assert_eq!(offset + SIZE_OF_SIGNATURE, shred.payload().len());
shred
.payload()
.get(..offset)
.unwrap_or_else(|| shred.payload())
}
get_payload(self) != get_payload(other)
}

fn retransmitter_signature_offset(&self) -> Result<usize, Error> {
match self {
Self::ShredCode(ShredCode::Merkle(shred)) => shred.retransmitter_signature_offset(),
Self::ShredData(ShredData::Merkle(shred)) => shred.retransmitter_signature_offset(),
Self::ShredCode(ShredCode::Legacy(_)) => Err(Error::InvalidShredVariant),
Self::ShredData(ShredData::Legacy(_)) => Err(Error::InvalidShredVariant),
}
}
}

// Helper methods to extract pieces of the shred from the payload
Expand Down Expand Up @@ -786,7 +817,7 @@ pub mod layout {
.ok_or(Error::InvalidPayloadSize(shred.len()))
}

pub(crate) fn set_retransmitter_signature(
pub fn set_retransmitter_signature(
shred: &mut [u8],
signature: &Signature,
) -> Result<(), Error> {
Expand Down
2 changes: 1 addition & 1 deletion ledger/src/shred/merkle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ macro_rules! impl_merkle_shred {
Ok(())
}

fn retransmitter_signature_offset(&self) -> Result<usize, Error> {
pub(super) fn retransmitter_signature_offset(&self) -> Result<usize, Error> {
let ShredVariant::$variant {
proof_size,
chained,
Expand Down

0 comments on commit 590a23c

Please sign in to comment.