Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

adds an optional encoding for serializing duplicate shreds #14362

Closed
wants to merge 2 commits into from

Conversation

behzadnouri
Copy link
Contributor

@behzadnouri behzadnouri commented Dec 30, 2020

Problem

The payload for DuplicateShred is larger than what can fit in one packet.
Currently this is split up into multiple chunks, each with its own added
overhead, Plus the overhead when these are wrapped in gossip messages:
https://github.com/solana-labs/solana/blob/cb8ba739a/core/src/duplicate_shred.rs#L32-L36
Compressing the payload may reduce the number of chunks needed to send one
complete duplicate slot proof over gossip.

Summary of Changes

  • Added an optional encoding for duplicate shred payloads.
  • Since decoding is done in a separate thread than gossip, this should not slow
    down gossip.
  • The test shows ~35% reduction in payload size of duplicate shreds.

@codecov
Copy link

codecov bot commented Dec 31, 2020

Codecov Report

Merging #14362 (4fd283e) into master (dcaa025) will decrease coverage by 0.0%.
The diff coverage is 87.1%.

@@            Coverage Diff            @@
##           master   #14362     +/-   ##
=========================================
- Coverage    80.3%    80.3%   -0.1%     
=========================================
  Files         403      404      +1     
  Lines      101131   101219     +88     
=========================================
+ Hits        81283    81346     +63     
- Misses      19848    19873     +25     

@behzadnouri behzadnouri force-pushed the encode branch 2 times, most recently from 09be5db to 3952f32 Compare December 31, 2020 15:33
@behzadnouri behzadnouri marked this pull request as ready for review December 31, 2020 16:01
Encoded::Deflate(bytes) => Box::new(DeflateDecoder::new(&bytes[..])),
Encoded::Gzip(bytes) => Box::new(GzDecoder::new(&bytes[..])),
Encoded::Zlib(bytes) => Box::new(ZlibDecoder::new(&bytes[..])),
Encoded::Zstd(bytes) => Box::new(zstd::stream::read::Decoder::new(&bytes[..])?),
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these decoders hardened against zip-bomb-like behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I raised that question to the upstream package; and meanwhile, added a limit here for number of bytes decoded.

@behzadnouri behzadnouri force-pushed the encode branch 2 times, most recently from a39eb29 to 9c4acc2 Compare January 7, 2021 20:02
@behzadnouri behzadnouri force-pushed the encode branch 2 times, most recently from fd1c189 to 1d1b6f5 Compare January 14, 2021 14:33
@stale
Copy link

stale bot commented Jan 26, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale [bot only] Added to stale content; results in auto-close after a week. label Jan 26, 2021
@stale
Copy link

stale bot commented Feb 7, 2021

This stale pull request has been automatically closed. Thank you for your contributions.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
stale [bot only] Added to stale content; results in auto-close after a week.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants