removes redundant shred.sanitize() from blockstore (#28016)

Shreds received from other nodes over the socket are sanitized when the
payload is deserialized:
https://github.com/solana-labs/solana/blob/315707504/ledger/src/shred/legacy.rs#L137
https://github.com/solana-labs/solana/blob/315707504/ledger/src/shred/legacy.rs#L77
https://github.com/solana-labs/solana/blob/315707504/ledger/src/shred/merkle.rs#L355
https://github.com/solana-labs/solana/blob/315707504/ledger/src/shred/merkle.rs#L439

Similarly, shreds recovered from erasure codes are also sanitized at
deserialization:
https://github.com/solana-labs/solana/blob/f02fe9c7e/ledger/src/shredder.rs#L330
or explicitly so for Merkle shreds:
https://github.com/solana-labs/solana/blob/f02fe9c7e/ledger/src/shred/merkle.rs#L753

Shreds generated locally by the node itself during its leader slots do
not need to be sanitized.

So sanitizing shreds in blockstore is redundant and wasteful. In
particular this becomes more wasteful with Merkle shreds because
sanitizing shreds would require verifying Merkle proof.
As such the commit removes redundant shred.sanitize() from blockstore.
This commit is contained in:
behzad nouri 2022-09-24 16:31:50 +00:00 committed by GitHub
parent f02fe9c7e7
commit 45e26574f3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 10 additions and 33 deletions

View File

@ -10,6 +10,7 @@ documentation = "https://docs.rs/solana-ledger"
edition = "2021"
[dependencies]
assert_matches = "1.5.0"
bincode = "1.3.3"
bitflags = "1.3.1"
byteorder = "1.4.3"
@ -68,7 +69,6 @@ default-features = false
features = ["lz4"]
[dev-dependencies]
assert_matches = "1.5.0"
bs58 = "0.4.0"
matches = "0.1.9"
solana-account-decoder = { path = "../account-decoder", version = "=1.15.0" }

View File

@ -22,6 +22,7 @@ use {
},
slot_stats::{ShredSource, SlotsStats},
},
assert_matches::debug_assert_matches,
bincode::deserialize,
crossbeam_channel::{bounded, Receiver, Sender, TrySendError},
dashmap::DashSet,
@ -1363,7 +1364,8 @@ impl Blockstore {
}
fn should_insert_coding_shred(shred: &Shred, last_root: &RwLock<u64>) -> bool {
shred.is_code() && shred.sanitize().is_ok() && shred.slot() > *last_root.read().unwrap()
debug_assert_matches!(shred.sanitize(), Ok(()));
shred.is_code() && shred.slot() > *last_root.read().unwrap()
}
fn insert_coding_shred(
@ -1377,7 +1379,8 @@ impl Blockstore {
// Assert guaranteed by integrity checks on the shred that happen before
// `insert_coding_shred` is called
assert!(shred.is_code() && shred.sanitize().is_ok());
debug_assert_matches!(shred.sanitize(), Ok(()));
assert!(shred.is_code());
// Commit step: commit all changes to the mutable structures at once, or none at all.
// We don't want only a subset of these changes going through.
@ -1426,23 +1429,7 @@ impl Blockstore {
} else {
false
};
if let Err(err) = shred.sanitize() {
let leader_pubkey = leader_schedule
.and_then(|leader_schedule| leader_schedule.slot_leader_at(slot, None));
datapoint_error!(
"blockstore_error",
(
"error",
format!(
"Leader {:?}, slot {}: received invalid shred: {:?}",
leader_pubkey, slot, err,
),
String
)
);
return false;
}
debug_assert_matches!(shred.sanitize(), Ok(()));
// Check that we do not receive shred_index >= than the last_index
// for the slot
let last_index = slot_meta.last_index;
@ -6311,18 +6298,6 @@ pub mod tests {
&last_root
));
// Trying to insert a shred with index < position should fail
{
let mut coding_shred = coding_shred.clone();
let index = coding_shred.index() - coding_shred.fec_set_index() - 1;
coding_shred.set_index(index as u32);
assert!(!Blockstore::should_insert_coding_shred(
&coding_shred,
&last_root
));
}
// Trying to insert value into slot <= than last root should fail
{
let mut coding_shred = coding_shred.clone();

View File

@ -15,6 +15,7 @@ use {
},
shredder::{self, ReedSolomon},
},
assert_matches::debug_assert_matches,
itertools::Itertools,
rayon::{prelude::*, ThreadPool},
reed_solomon_erasure::Error::{InvalidIndex, TooFewParityShards, TooFewShards},
@ -996,7 +997,7 @@ fn make_erasure_batch(
shred.set_merkle_branch(merkle_branch)?;
shred.set_signature(signature);
debug_assert!(shred.verify(&keypair.pubkey()));
debug_assert!(shred.sanitize().is_ok());
debug_assert_matches!(shred.sanitize(), Ok(()));
// Assert that shred payload is fully populated.
debug_assert_eq!(shred, {
let shred = shred.payload().clone();

View File

@ -4964,6 +4964,7 @@ dependencies = [
name = "solana-ledger"
version = "1.15.0"
dependencies = [
"assert_matches",
"bincode",
"bitflags",
"byteorder 1.4.3",