Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

Cli configurable validators #18630

Merged
merged 2 commits into from
Jul 14, 2021

Conversation

t-nelson
Copy link
Contributor

Problem

solana validators can only return a filtered list of validators

Summary of Changes

  • add config parameters for getVoteAccounts
  • plumb them through CLI

@t-nelson t-nelson requested a review from CriesofCarrots July 13, 2021 06:21
@t-nelson t-nelson force-pushed the cli-configurable-validators branch from d1a6f7d to e64b612 Compare July 13, 2021 06:35
@codecov
Copy link

codecov bot commented Jul 13, 2021

Codecov Report

Merging #18630 (d612c5a) into master (74b29c0) will decrease coverage by 0.0%.
The diff coverage is 55.1%.

@@            Coverage Diff            @@
##           master   #18630     +/-   ##
=========================================
- Coverage    82.7%    82.7%   -0.1%     
=========================================
  Files         436      436             
  Lines      123485   123512     +27     
=========================================
- Hits       102192   102168     -24     
- Misses      21293    21344     +51     

Copy link
Contributor

@CriesofCarrots CriesofCarrots left a comment

Choose a reason for hiding this comment

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

Code lgtm, one nit.
Concerned at all about the implications of variable delinquent slot distance for downstream tools (block explorers, validator stats, etc)? Or you figure that anyone assertive enough to use delinquent_slot_distance should expose that configuration to users in their product?

rpc/src/rpc.rs Outdated Show resolved Hide resolved
cli/src/cluster_query.rs Show resolved Hide resolved
@t-nelson
Copy link
Contributor Author

Code lgtm, one nit.
Concerned at all about the implications of variable delinquent slot distance for downstream tools (block explorers, validator stats, etc)? Or you figure that anyone assertive enough to use delinquent_slot_distance should expose that configuration to users in their product?

I'd like to think that no one would bother to expose it at all. This does remind me that I should update the docs though, so I'll put a warning there.

@t-nelson t-nelson force-pushed the cli-configurable-validators branch from e64b612 to d680cd4 Compare July 14, 2021 06:44
@t-nelson t-nelson added the automerge Merge this Pull Request automatically once CI passes label Jul 14, 2021
@mergify mergify bot removed the automerge Merge this Pull Request automatically once CI passes label Jul 14, 2021
@mergify
Copy link
Contributor

mergify bot commented Jul 14, 2021

automerge label removed due to a CI failure

@t-nelson t-nelson force-pushed the cli-configurable-validators branch from d680cd4 to d612c5a Compare July 14, 2021 06:57
@t-nelson t-nelson added the automerge Merge this Pull Request automatically once CI passes label Jul 14, 2021
@mergify mergify bot merged commit a4a24b6 into solana-labs:master Jul 14, 2021
@CriesofCarrots
Copy link
Contributor

I'd like to think that no one would bother to expose it at all

If "http://validator-website.com" is calling my node delinquent for being 20 slots behind, but my node looks fine on "http://validator-website-2.com" because they are using a threshold of 300 slots, I would want to know?

@t-nelson t-nelson deleted the cli-configurable-validators branch July 14, 2021 21:04
@t-nelson
Copy link
Contributor Author

I'd like to think that no one would bother to expose it at all

If "http://validator-website.com" is calling my node delinquent for being 20 slots behind, but my node looks fine on "http://validator-website-2.com" because they are using a threshold of 300 slots, I would want to know?

Sorry, I misread that as expose as a knob to the user. I'd really like to think no one would pass it, period. The following note of discouragement was added with the docs update. Hopefully it's sufficient

- (optional) `delinquentSlotDistance: <u64>` - Specify the number of slots behind the tip that a validator must fall to be considered delinquent. **NOTE:** For the sake of consistency between ecosystem products, _it is **not** recommended that this argument be specified._

mergify bot added a commit that referenced this pull request Jul 16, 2021
* rpc: more params for `GetVoteAccountsConfig`

(cherry picked from commit bf90ea2)

# Conflicts:
#	docs/src/developing/clients/jsonrpc-api.md

* cli: allow returning more `solana validators`

(cherry picked from commit a4a24b6)

# Conflicts:
#	Cargo.lock
#	cli/Cargo.toml
#	cli/src/cluster_query.rs

Co-authored-by: Trent Nelson <trent@solana.com>
mergify bot added a commit that referenced this pull request Jul 16, 2021
* rpc: more params for `GetVoteAccountsConfig`

(cherry picked from commit bf90ea2)

* cli: allow returning more `solana validators`

(cherry picked from commit a4a24b6)

# Conflicts:
#	Cargo.lock

Co-authored-by: Trent Nelson <trent@solana.com>
This was referenced Aug 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
automerge Merge this Pull Request automatically once CI passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants