verifies shred-version in fetch stage #26070

behzadnouri · 2022-06-19T18:21:56Z

Problem

Shred versions are not verified until window-service where resources are
already wasted to sig-verify and deserialize shreds.

Summary of Changes

The commit verifies shred-version earlier in the pipeline in fetch stage.

behzadnouri · 2022-06-19T18:26:50Z

core/src/window_service.rs

-        } else if shred.version() != shred_version {
-            inc_new_counter_debug!("streamer-recv_window-incorrect_shred_version", 1);
-            false
-        } else if shred.index() >= MAX_DATA_SHREDS_PER_SLOT as u32 {
-            inc_new_counter_warn!("streamer-recv_window-shred_index_overrun", 1);
-            false
-        } else if shred.sanitize().is_err() {
-            inc_new_counter_warn!("streamer-recv_window-invalid-shred", 1);
-            false


} else if shred.version() != shred_version {

moved earlier in the pipeline in the shred-fetch stage.

} else if shred.index() >= MAX_DATA_SHREDS_PER_SLOT as u32 {

is already checked in shred.sanitize().

} else if shred.sanitize().is_err() {

shreds are already sanitized when de-serialized.

behzadnouri · 2022-06-19T18:30:35Z

core/src/window_service.rs

@@ -187,15 +186,6 @@ pub(crate) fn should_retransmit_and_persist(
        } else if !verify_shred_slot(shred, root) {
            inc_new_counter_debug!("streamer-recv_window-outdated_transmission", 1);
            false


will follow up with moving this to fetch-stage as well.

Shred versions are not verified until window-service where resources are already wasted to sig-verify and deserialize shreds. The commit verifies shred-version earlier in the pipeline in fetch stage.

... in favor of just passing packet flags.

carllin · 2022-06-22T04:27:56Z

core/src/shred_fetch_stage.rs

+        stats.shred_version_mismatch += 1;
+        return true;
+    }
+    let hash = packet_hasher.hash_packet(packet);
+    match shreds_received.put(hash, ()) {
+        None => false,
+        Some(()) => {
+            stats.duplicate_shred += 1;


Should we be using checked math here?

I just moved the old code here.
I don't think we can ingest usize::max packets in one or two seconds anyways, but nonetheless I will revisit this in follow up changes.

solana-labs#26070 introduced MAX_CODE_SHREDS_PER_SLOT which because of how coding shreds are generated was later set larger than MAX_DATA_SHREDS_PER_SLOT. The old code was discarding: index >= MAX_DATA_SHREDS_PER_SLOT regardless of if the shred is code or data: https://github.com/solana-labs/solana/blob/v1.13.6/core/src/window_service.rs#L193-L195 Since that many code (or data) shreds will probably never result in a rooted slot anyways, the commit reduces MAX_CODE_SHREDS_PER_SLOT to what previously was the effective limit.

#26070 introduced MAX_CODE_SHREDS_PER_SLOT which because of how coding shreds are generated was later set larger than MAX_DATA_SHREDS_PER_SLOT. The old code was discarding: index >= MAX_DATA_SHREDS_PER_SLOT regardless of if the shred is code or data: https://github.com/solana-labs/solana/blob/v1.13.6/core/src/window_service.rs#L193-L195 Since that many code (or data) shreds will probably never result in a rooted slot anyways, the commit reduces MAX_CODE_SHREDS_PER_SLOT to what previously was the effective limit.

#26070 introduced MAX_CODE_SHREDS_PER_SLOT which because of how coding shreds are generated was later set larger than MAX_DATA_SHREDS_PER_SLOT. The old code was discarding: index >= MAX_DATA_SHREDS_PER_SLOT regardless of if the shred is code or data: https://github.com/solana-labs/solana/blob/v1.13.6/core/src/window_service.rs#L193-L195 Since that many code (or data) shreds will probably never result in a rooted slot anyways, the commit reduces MAX_CODE_SHREDS_PER_SLOT to what previously was the effective limit. (cherry picked from commit 06a8419)

reduces MAX_CODE_SHREDS_PER_SLOT (#30543) #26070 introduced MAX_CODE_SHREDS_PER_SLOT which because of how coding shreds are generated was later set larger than MAX_DATA_SHREDS_PER_SLOT. The old code was discarding: index >= MAX_DATA_SHREDS_PER_SLOT regardless of if the shred is code or data: https://github.com/solana-labs/solana/blob/v1.13.6/core/src/window_service.rs#L193-L195 Since that many code (or data) shreds will probably never result in a rooted slot anyways, the commit reduces MAX_CODE_SHREDS_PER_SLOT to what previously was the effective limit. (cherry picked from commit 06a8419) Co-authored-by: behzad nouri <behzadnouri@gmail.com>

solana-labs#26070 introduced MAX_CODE_SHREDS_PER_SLOT which because of how coding shreds are generated was later set larger than MAX_DATA_SHREDS_PER_SLOT. The old code was discarding: index >= MAX_DATA_SHREDS_PER_SLOT regardless of if the shred is code or data: https://github.com/solana-labs/solana/blob/v1.13.6/core/src/window_service.rs#L193-L195 Since that many code (or data) shreds will probably never result in a rooted slot anyways, the commit reduces MAX_CODE_SHREDS_PER_SLOT to what previously was the effective limit.

behzadnouri commented Jun 19, 2022

View reviewed changes

behzadnouri force-pushed the shred-fetch-sanitize-lite branch from 7f43a18 to 2b50afc Compare June 19, 2022 18:29

behzadnouri commented Jun 19, 2022

View reviewed changes

behzadnouri requested review from steviez, sakridge, carllin and jbiseda June 19, 2022 18:31

verifies shred-version in fetch stage

9bf09c6

Shred versions are not verified until window-service where resources are already wasted to sig-verify and deserialize shreds. The commit verifies shred-version earlier in the pipeline in fetch stage.

behzadnouri force-pushed the shred-fetch-sanitize-lite branch 2 times, most recently from 84b006b to 0adb8ed Compare June 20, 2022 20:30

removes packet modifier from shred_fetch_stage

dc57aef

... in favor of just passing packet flags.

behzadnouri force-pushed the shred-fetch-sanitize-lite branch from 0adb8ed to dc57aef Compare June 20, 2022 22:19

carllin approved these changes Jun 22, 2022

View reviewed changes

carllin reviewed Jun 22, 2022

View reviewed changes

behzadnouri merged commit faa6c32 into solana-labs:master Jun 22, 2022

behzadnouri deleted the shred-fetch-sanitize-lite branch June 22, 2022 12:17

behzadnouri mentioned this pull request Feb 27, 2023

reduces MAX_CODE_SHREDS_PER_SLOT #30543

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

verifies shred-version in fetch stage #26070

verifies shred-version in fetch stage #26070

behzadnouri commented Jun 19, 2022

behzadnouri Jun 19, 2022 •

edited

Loading

behzadnouri Jun 19, 2022 •

edited

Loading

carllin Jun 22, 2022

behzadnouri Jun 22, 2022

verifies shred-version in fetch stage #26070

verifies shred-version in fetch stage #26070

Conversation

behzadnouri commented Jun 19, 2022

Problem

Summary of Changes

behzadnouri Jun 19, 2022 • edited Loading

Choose a reason for hiding this comment

behzadnouri Jun 19, 2022 • edited Loading

Choose a reason for hiding this comment

carllin Jun 22, 2022

Choose a reason for hiding this comment

behzadnouri Jun 22, 2022

Choose a reason for hiding this comment

behzadnouri Jun 19, 2022 •

edited

Loading

behzadnouri Jun 19, 2022 •

edited

Loading