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

dedups shreds by common-header instead of the entire payload #4187

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

behzadnouri
Copy link

Problem

Shreds in the retransmit stage:

  • don't have repair nonce (repaired shreds are not retransmitted).
  • are already resigned by this node as the retransmitter.
  • have their leader's signature verified.

Therefore in order to dedup shreds, it suffices to compare:

(signature, slot, shred-index, shred-type)

Because ShredCommonHeader already includes all of the above tuple, the rest of the payload can be skipped.

Summary of Changes

The commit dedups shreds by common-header and skips the rest of the payload.

@behzadnouri behzadnouri requested review from AshwinSekar and removed request for AshwinSekar December 20, 2024 02:01
@behzadnouri behzadnouri changed the title dedups shreds by common-header instead of entire payload dedups shreds by common-header instead of the entire payload Dec 20, 2024
@behzadnouri behzadnouri force-pushed the shred-dedup-common-header branch from 6686e09 to c0f556d Compare December 20, 2024 13:34
Shreds in the retransmit stage:
  * don't have repair nonce (repaired shreds are not retransmitted).
  * are already resigned by this node as the retransmitter.
  * have their leader's signature verified.
Therefore in order to dedup shreds, it suffices to compare:

    (signature, slot, shred-index, shred-type)

Because ShredCommonHeader already includes all of the above tuple,
the rest of the payload can be skipped.
@behzadnouri behzadnouri force-pushed the shred-dedup-common-header branch from c0f556d to e151482 Compare December 20, 2024 13:37
Copy link

@AshwinSekar AshwinSekar left a comment

Choose a reason for hiding this comment

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

LGTM, the signature is on the merkle root which already contains the payload so this should suffice.

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.

2 participants