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 api to obtain the parent node in the turbine retransmit tree #115

Merged
merged 1 commit into from
Mar 7, 2024

Conversation

behzadnouri
Copy link

Problem

Lock down turbine propagation tree.

Summary of Changes

Following commits will use this api to check retransmitter's signature on incoming shreds

@codecov-commenter
Copy link

codecov-commenter commented Mar 6, 2024

Codecov Report

Attention: Patch coverage is 75.00000% with 18 lines in your changes are missing coverage. Please review.

Project coverage is 81.8%. Comparing base (c161351) to head (4be7123).
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##           master     #115     +/-   ##
=========================================
- Coverage    81.8%    81.8%   -0.1%     
=========================================
  Files         838      838             
  Lines      225947   225998     +51     
=========================================
- Hits       184971   184966      -5     
- Misses      40976    41032     +56     

@behzadnouri behzadnouri marked this pull request as ready for review March 6, 2024 20:16
@behzadnouri behzadnouri force-pushed the get-retransmit-parent branch 2 times, most recently from 73d9dac to 4be7123 Compare March 6, 2024 20:54
@@ -589,10 +644,36 @@ mod tests {
}
}

fn check_retransmit_nodes(fanout: usize, nodes: &[usize], peers: Vec<Vec<usize>>) {
let index: HashMap<_, _> = nodes

Choose a reason for hiding this comment

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

might be helpful to name this something like node_to_turbine_index

Copy link
Author

Choose a reason for hiding this comment

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

added a comment.

Copy link

@bw-solana bw-solana left a comment

Choose a reason for hiding this comment

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

Seems like we could have separate tests for (1) verifying the indices returned by get_retransmit_peers against expected indices and (2) verifying parent --> child --> parent translations are working. If we trust 1, we should be able to generically sweep across different degrees of fanout and numbers of nodes fairly easily

Following commits will use this api to check retransmitter's signature
on incoming shreds.
@behzadnouri
Copy link
Author

Seems like we could have separate tests for (1) verifying the indices returned by get_retransmit_peers against expected indices and (2) verifying parent --> child --> parent translations are working. If we trust 1, we should be able to generically sweep across different degrees of fanout and numbers of nodes fairly easily

added a round trip test to verify the two function invert each other.

Copy link

@bw-solana bw-solana left a comment

Choose a reason for hiding this comment

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

LGTM

@behzadnouri behzadnouri merged commit 42e8309 into anza-xyz:master Mar 7, 2024
46 checks passed
@behzadnouri behzadnouri deleted the get-retransmit-parent branch March 7, 2024 17:56
Copy link

mergify bot commented Mar 7, 2024

Backports to the beta branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule. Exceptions include CI/metrics changes, CLI improvements and documentation updates on a case by case basis.

mergify bot pushed a commit that referenced this pull request Mar 7, 2024
Following commits will use this api to check retransmitter's signature
on incoming shreds.

(cherry picked from commit 42e8309)
mergify bot added a commit that referenced this pull request Mar 8, 2024
…ree (backport of #115) (#135)

adds api to obtain the parent node in the turbine retransmit tree (#115)

Following commits will use this api to check retransmitter's signature
on incoming shreds.

(cherry picked from commit 42e8309)

Co-authored-by: behzad nouri <[email protected]>
anwayde pushed a commit to firedancer-io/agave that referenced this pull request Jul 23, 2024
…ree (backport of anza-xyz#115) (anza-xyz#135)

adds api to obtain the parent node in the turbine retransmit tree (anza-xyz#115)

Following commits will use this api to check retransmitter's signature
on incoming shreds.

(cherry picked from commit 42e8309)

Co-authored-by: behzad nouri <[email protected]>
codebender828 pushed a commit to codebender828/agave that referenced this pull request Oct 3, 2024
…za-xyz#115)

Following commits will use this api to check retransmitter's signature
on incoming shreds.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants