Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adds methods to obtain shreds' erasure coded block and index #21325

Merged
merged 1 commit into from Nov 19, 2021

Conversation

behzadnouri
Copy link
Contributor

Problem

Working towards encapsulating erasure coding scheme inside shreds in order to simplify upcoming changes and not to leak implementation details.

Summary of Changes

  • Added methods to obtain shreds' erasure coded block and index.
  • More sanity checks when computing erasure block index.
  • Avoiding panic in .drain.

@codecov
Copy link

codecov bot commented Nov 17, 2021

Codecov Report

Merging #21325 (a08b17e) into master (c3e5927) will decrease coverage by 0.0%.
The diff coverage is 87.0%.

@@            Coverage Diff            @@
##           master   #21325     +/-   ##
=========================================
- Coverage    81.5%    81.5%   -0.1%     
=========================================
  Files         500      501      +1     
  Lines      140604   141003    +399     
=========================================
+ Hits       114704   114981    +277     
- Misses      25900    26022    +122     

let fec_set_index = self.common_header.fec_set_index;
let index = self.index().checked_sub(fec_set_index)? as usize;
if self.is_data() {
Some(index)
Copy link
Contributor

@carllin carllin Nov 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also uphold this existing check from the current code:

if index >= num_data_shreds {
     return Err(InvalidIndex);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't know number of data shreds here, and I don't want to pass it in as a function argument (or check it inside Shredder) because then that would leak out implementation details.
The goal of this commit was to encapsulate and hide the implementation details of erasure coding indices.

I think it is fine not to check for it because data reconstruct from codes will error out anyways.

The check was only added recently in #21082
and is not released yet.

@behzadnouri behzadnouri merged commit 7da2df7 into solana-labs:master Nov 19, 2021
@behzadnouri behzadnouri deleted the erasure-block branch November 19, 2021 20:01
dankelleher pushed a commit to identity-com/solana that referenced this pull request Nov 24, 2021
frits-metalogix added a commit to identity-com/solana that referenced this pull request Nov 24, 2021
mergify bot pushed a commit that referenced this pull request Jan 14, 2022
mergify bot added a commit that referenced this pull request Jan 14, 2022
…#22508)

(cherry picked from commit 7da2df7)

Co-authored-by: behzad nouri <behzadnouri@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants