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

Show more detailed vote history in shorter format#13374

Merged
ryoqun merged 3 commits intosolana-labs:masterfrom
ryoqun:more-detailed-vote-history
Nov 6, 2020
Merged

Show more detailed vote history in shorter format#13374
ryoqun merged 3 commits intosolana-labs:masterfrom
ryoqun:more-detailed-vote-history

Conversation

@ryoqun
Copy link
Copy Markdown
Contributor

@ryoqun ryoqun commented Nov 3, 2020

Problem

Sometimes we want to view human-friendly raw data of vote accounts. But, solana vote-account is too kind to show that level of detailed information. It only shows some computed information.

Summary of Changes

Show everything, also tidy up formatting a bit, inspired by solana epoch-info.

before

Epoch Voting History:
- epoch: 90
  slots in epoch: 432000
  credits earned: 376266
- epoch: 91
  slots in epoch: 432000
  credits earned: 431241
- epoch: 92
  slots in epoch: 432000
  credits earned: 387252

after

Epoch Voting History:
* missed credits include slots unavailable to vote on due to delinquent leaders
- epoch: 90
  credits range: [0..376266)
  credits/slots: 376266/432000
- epoch: 91
  credits range: [376266..807507)
  credits/slots: 431241/432000
- epoch: 92
  credits range: [807507..1194759)
  credits/slots: 387252/432000

Context #13233

@ryoqun ryoqun requested a review from CriesofCarrots November 3, 2020 23:20
writeln!(
f,
" credits range: [{}..{})",
entry.prev_credits, entry.credits
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Especially, I needed this info.

writeln!(
f,
" credits/slots: {}/{}",
entry.credits_earned, entry.slots_in_epoch
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Imply that credits here cannot be larger than slots. Also removed extra adjectives because the context should clarify them.

Comment thread cli-output/src/cli_output.rs Outdated
writeln!(f, "- slot: {}", vote.slot)?;
writeln!(f, " confirmation count: {}", vote.confirmation_count)?;
}
writeln!(f, "Epoch Voting History (without skipped slots):")?;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Actually, I wanted to show skipped slots but it's not easy task. So, at least show a note about it to educate/inform users correctly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

tried to be shorter but this might be too ambuguious. (skipped slots not considered)/(missed credits can be skipped slots) might be better?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I do find that language a little confusing.
Maybe something like this on a new line? * not all epoch slots may have been available to vote on

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@CriesofCarrots Thanks for suggestion! I've further adjusted the wording based on yours to make interested users be able to have a clue to know more: 14bdcba How does this look?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Much better. Thanks for the polish!

@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 4, 2020

Codecov Report

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

@@           Coverage Diff           @@
##           master   #13374   +/-   ##
=======================================
  Coverage    82.0%    82.0%           
=======================================
  Files         376      376           
  Lines       87888    87901   +13     
=======================================
+ Hits        72107    72120   +13     
  Misses      15781    15781           

@ryoqun ryoqun merged commit d08c323 into solana-labs:master Nov 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants