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

passes through feature-set to gossip requests handling #12878

Merged
merged 2 commits into from
Oct 15, 2020

Conversation

behzadnouri
Copy link
Contributor

Problem

For #12794 we want to activate ping/pong check of addresses dynamically once super majority of nodes have updated to the new code. see: #12794 (comment)

Summary of Changes

The commit adds a new feature id for ping/pong check and passes through the feature-set construct down to request handler.

@behzadnouri behzadnouri changed the title passes throught feature-set to gossip requests handling passes through feature-set to gossip requests handling Oct 14, 2020
@behzadnouri behzadnouri force-pushed the feature-set branch 2 times, most recently from 48747f1 to 4a9d5c8 Compare October 14, 2020 18:33
@codecov
Copy link

codecov bot commented Oct 14, 2020

Codecov Report

Merging #12878 into master will decrease coverage by 0.0%.
The diff coverage is 81.3%.

@@            Coverage Diff            @@
##           master   #12878     +/-   ##
=========================================
- Coverage    81.9%    81.9%   -0.1%     
=========================================
  Files         360      361      +1     
  Lines       84925    85057    +132     
=========================================
+ Hits        69580    69682    +102     
- Misses      15345    15375     +30     

@behzadnouri behzadnouri marked this pull request as ready for review October 14, 2020 19:47
@behzadnouri behzadnouri requested a review from mvines October 14, 2020 19:48
mvines
mvines previously approved these changes Oct 15, 2020
bank_forks
.read()
.unwrap()
.working_bank()
Copy link
Member

Choose a reason for hiding this comment

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

It'll be better to use .root_bank(). With the .working_bank(), we could see a feature get enabled and then disabled again (if the feature happens to get enabled in a minority fork). Otherwise lgtm!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

There are other places in this file which are using .working_bank (e.g. for stakes). Do you think they should also use .root_bank instead?

Copy link
Member

Choose a reason for hiding this comment

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

I'd say so, but only for consistency. It looks like it shouldn't matter in practice either way though

@mergify mergify bot dismissed mvines’s stale review October 15, 2020 18:30

Pull request has been modified.

@behzadnouri behzadnouri merged commit 4828316 into solana-labs:master Oct 15, 2020
@behzadnouri behzadnouri deleted the feature-set branch October 15, 2020 20:54
carllin pushed a commit to carllin/solana that referenced this pull request Oct 17, 2020
…2878)

* passes through feature-set to down to gossip requests handling
* takes the feature-set from root_bank instead of working_bank
mergify bot pushed a commit that referenced this pull request Oct 19, 2020
* passes through feature-set to down to gossip requests handling
* takes the feature-set from root_bank instead of working_bank

(cherry picked from commit 4828316)
jackcmay pushed a commit that referenced this pull request Oct 20, 2020
* passes through feature-set to down to gossip requests handling
* takes the feature-set from root_bank instead of working_bank

(cherry picked from commit 4828316)
jackcmay pushed a commit that referenced this pull request Oct 20, 2020
* passes through feature-set to down to gossip requests handling
* takes the feature-set from root_bank instead of working_bank

(cherry picked from commit 4828316)
mergify bot added a commit that referenced this pull request Oct 20, 2020
* passes through feature-set to down to gossip requests handling
* takes the feature-set from root_bank instead of working_bank

(cherry picked from commit 4828316)

Co-authored-by: behzad nouri <[email protected]>
mergify bot pushed a commit that referenced this pull request Oct 27, 2020
* passes through feature-set to down to gossip requests handling
* takes the feature-set from root_bank instead of working_bank

(cherry picked from commit 4828316)
mergify bot added a commit that referenced this pull request Oct 27, 2020
* passes through feature-set to down to gossip requests handling
* takes the feature-set from root_bank instead of working_bank

(cherry picked from commit 4828316)

Co-authored-by: behzad nouri <[email protected]>
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