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.

Sign up for GitHub

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

Jump to bottom

Add commitment Root variant, and add fleshed out --commitment arg to Cli #9806

Merged
merged 8 commits into from
Apr 30, 2020
Merged

Add commitment Root variant, and add fleshed out --commitment arg to Cli #9806

merged 8 commits into from
Apr 30, 2020

Conversation

CriesofCarrots
Copy link
Contributor

Problem

We recently redefined max commitment to mean the most recent block that a supermajority of the cluster has rooted, which involved using this block in rpc by default. However, there are times when you want to get data from a node root and the cluster-confirmed root is problematic: for instance, if your node is many blocks behind the cluster.

Summary of Changes

  • Add CommitmentLevel::Root variant, and plumb into rpc to use BankForks::root() Bank
  • Replace Cli --confirmed args with --commitment=default|max|recent|root, retaining the default to recent that was implemented for those subcommands

As discussed: #9750 (comment)

@CriesofCarrots
Copy link
Contributor Author

@mvines , are any of the --confirmed queries being used in scripts elsewhere that you're aware of?

@mvines
Copy link
Member

mvines commented Apr 30, 2020

@mvines , are any of the --confirmed queries being used in scripts elsewhere that you're aware of?

Nothing comes to mind, we'll find them once this lands though :)

mvines
mvines previously approved these changes Apr 30, 2020
View reviewed changes
Copy link
Member

@mvines mvines left a comment

Choose a reason for hiding this comment

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

sweet, I love the cli code deduping

@mergify mergify bot dismissed mvines’s stale review April 30, 2020 03:30

Pull request has been modified.

@CriesofCarrots CriesofCarrots added the automerge Merge this Pull Request automatically once CI passes label Apr 30, 2020
@codecov
Copy link

codecov bot commented Apr 30, 2020

Codecov Report

Merging #9806 into master will increase coverage by 0.0%.
The diff coverage is 86.9%.

@@          Coverage Diff           @@
##           master   #9806   +/-   ##
======================================
  Coverage    80.4%   80.4%           
======================================
  Files         281     282    +1     
  Lines       64644   64635    -9     
======================================
+ Hits        51975   51984    +9     
+ Misses      12669   12651   -18

@solana-grimes solana-grimes merged commit 6deaf64 into solana-labs:master Apr 30, 2020
mergify bot pushed a commit that referenced this pull request Apr 30, 2020
@CriesofCarrots
Add commitment Root variant, and add fleshed out --commitment arg to …
0e5985d 
…Cli (#9806)

automerge

(cherry picked from commit 6deaf64)

# Conflicts:
#	cli/src/cluster_query.rs
#	cli/src/vote.rs
@mergify mergify bot mentioned this pull request Apr 30, 2020
Merged
mergify bot pushed a commit that referenced this pull request Apr 30, 2020
@CriesofCarrots
Add commitment Root variant, and add fleshed out --commitment arg to …
642ab6c 
…Cli (#9806)

automerge

(cherry picked from commit 6deaf64)

# Conflicts:
#	cli/src/vote.rs
@mergify mergify bot mentioned this pull request Apr 30, 2020
Merged
CriesofCarrots added a commit that referenced this pull request Apr 30, 2020
@CriesofCarrots
Add commitment Root variant, and add fleshed out --commitment arg to …
bdb0bf0 
…Cli (#9806)

automerge

(cherry picked from commit 6deaf64)
CriesofCarrots added a commit that referenced this pull request Apr 30, 2020
@CriesofCarrots
Add commitment Root variant, and add fleshed out --commitment arg to …
c9cd473 
…Cli (#9806)

automerge

(cherry picked from commit 6deaf64)
solana-grimes pushed a commit that referenced this pull request Apr 30, 2020
@mergify
Add commitment Root variant, and add fleshed out --commitment arg to …
3f41d60 
…Cli (#9806) (#9813)

automerge
solana-grimes pushed a commit that referenced this pull request Apr 30, 2020
@mergify
Add commitment Root variant, and add fleshed out --commitment arg to …
c63bd05 
…Cli (#9806) (#9812)

automerge
@CriesofCarrots CriesofCarrots deleted the commitment-root branch May 1, 2020 23:37
danpaul000 pushed a commit to danpaul000/solana that referenced this pull request Jul 8, 2020
@CriesofCarrots
Add commitment Root variant, and add fleshed out --commitment arg to …
71046a6 
…Cli (solana-labs#9806)

automerge
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mvines mvines mvines left review comments

Assignees
No one assigned
Labels
automerge Merge this Pull Request automatically once CI passes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@CriesofCarrots @mvines @solana-grimes