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

Turbine needs to be more deterministic #7043

Closed
4 tasks
sagar-solana opened this issue Nov 19, 2019 · 7 comments
Closed
4 tasks

Turbine needs to be more deterministic #7043

sagar-solana opened this issue Nov 19, 2019 · 7 comments
Assignees
Labels
security Pull requests that address a security vulnerability
Milestone

Comments

@sagar-solana
Copy link
Contributor

sagar-solana commented Nov 19, 2019

Problem

The computed Turbine tree is not deterministic.
It uses the gossip peer list to decide where shreds are transmitted.
This peer list might be different on different nodes and can cause issues if gossip has been partitioned in a more targeted manner.

Proposed Solution

Instead of using the gossip list to filter down the broadcast set. Do the following in broadcast and in retransmit

  • Use the list of staked nodes in the epoch as the peer list
  • For any missing peers hold off on sending the shred (more on this later)
  • Potentially dial up erasure codes if shreds are skipped because of missing peers (does this solve anything? signatures might still cause shreds to point at missing peers)
  • In a separate thread/task broadcast the collected shreds (missing peers) and take on the role of each missing peer and complete the broadcast on their behalf.
@aeyakovenko aeyakovenko added the security Pull requests that address a security vulnerability label Nov 19, 2019
@aeyakovenko
Copy link
Member

DOS prevention/security depends on a determistic Turbine. Retransmits can only occur if the shred path as derived from the signature + epoch staked nodes indicates that the packet arrived from the leader for the node.

@sagar-solana
Copy link
Contributor Author

@aeyakovenko should I not dive into this right away?
If I don't should I make it so that we are only allowed to pull from these "old" contact infos instead of treating them as if they're live?

@aeyakovenko
Copy link
Member

@sagar-solana up to you, unless there is a better slp1 issue to take on. Ask @mvines :)

@mvines mvines added this to the v0.23.0 milestone Nov 25, 2019
@aeyakovenko
Copy link
Member

tag: @pgarg66

@mvines mvines modified the milestones: Tofino v0.23.0, Rincon v0.24.0 Jan 26, 2020
@mvines mvines modified the milestones: Rincon v0.24.0, v0.25.0 Feb 20, 2020
@mvines mvines modified the milestones: v1.1.0, v1.2.0 Mar 30, 2020
@mvines mvines modified the milestones: v1.2.0, The Future! Apr 20, 2020
@behzadnouri
Copy link
Contributor

Fixed by #18238, #18971 and #20480
Pending feature activation for the last one.

@behzadnouri behzadnouri self-assigned this Feb 16, 2022
@ryoqun
Copy link
Member

ryoqun commented May 13, 2022

@behzadnouri finally ripe time to close this old issue, right? (context: I'm doing spring cleaning for security issues)

@behzadnouri
Copy link
Contributor

@behzadnouri finally ripe time to close this old issue, right? (context: I'm doing spring cleaning for security issues)

yes, all related features are active on mainnet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
security Pull requests that address a security vulnerability
Projects
None yet
Development

No branches or pull requests

5 participants