Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 64 additions & 10 deletions cli-output/src/cli_output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use {
solana_stake_program::stake_state::{Authorized, Lockup},
solana_vote_program::{
authorized_voters::AuthorizedVoters,
vote_state::{BlockTimestamp, Lockout},
vote_state::{BlockTimestamp, Lockout, MAX_EPOCH_CREDITS_HISTORY, MAX_LOCKOUT_HISTORY},
},
std::{
collections::{BTreeMap, HashMap},
Expand Down Expand Up @@ -610,25 +610,63 @@ fn show_votes_and_credits(
return Ok(());
}

writeln!(f, "Recent Votes:")?;
for vote in votes {
writeln!(f, "- slot: {}", vote.slot)?;
writeln!(f, " confirmation count: {}", vote.confirmation_count)?;
Comment on lines -615 to -616
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.

Condensed this two lines to one for the precious vertical screen estate. :)

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.

NIT: What do you think about saving the per-line field name redundancy with a header line at the top?

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.

Hmm, I actually like this epoch-info-style (key: value). I might do this once I got enough motivation in the future. :)

}
writeln!(f, "Epoch Voting History:")?;
// Existence of this should guarantee the occurrence of vote truncation
let newest_history_entry = epoch_voting_history.iter().rev().next();

writeln!(
f,
"* missed credits include slots unavailable to vote on due to delinquent leaders",
"{} Votes (using {}/{} entries):",
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.

The subtle thing here is that 29/31 is possible even if vote truncation started because fork switch. So, the Recent/All title switching and this number indicator isn't strictly-speaking redundant information.

(if newest_history_entry.is_none() {
"All"
} else {
"Recent"
}),
votes.len(),
MAX_LOCKOUT_HISTORY
)?;
for entry in epoch_voting_history {

for vote in votes.iter().rev() {
writeln!(
f,
"- slot: {} (confirmation count: {})",
vote.slot, vote.confirmation_count
)?;
}
if let Some(newest) = newest_history_entry {
writeln!(
f,
"- ... (truncated {} rooted votes, which have been credited)",
newest.credits
)?;
}

if !epoch_voting_history.is_empty() {
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.

Well, originally, showed titles for no good reason even if is_empty()...

writeln!(
f,
"{} Epoch Voting History (using {}/{} entries):",
(if epoch_voting_history.len() < MAX_EPOCH_CREDITS_HISTORY {
"All"
} else {
"Recent"
}),
Comment on lines +647 to +651
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.

Made this section dynamic; similar treatment was applied to the votes section for consistentcy. ... so that we can quickly determine the existence of truncation by looking either at the top or bottom of possibly long list. :)

epoch_voting_history.len(),
MAX_EPOCH_CREDITS_HISTORY
)?;
writeln!(
f,
"* missed credits include slots unavailable to vote on due to delinquent leaders",
)?;
}

for entry in epoch_voting_history.iter().rev() {
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.

NIT: Maybe give this block the single-line treatment as well?

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.

well, I rather keep this as-is. Maybe, the single-line style condenses too much information in a single line.

writeln!(
f, // tame fmt so that this will be folded like following
"- epoch: {}",
entry.epoch
)?;
writeln!(
f,
" credits range: [{}..{})",
" credits range: ({}..{}]",
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.

as this run.sh sample shows this is more proper range notation... I was wrong at #13374 in hindsight after more deeper understanding of staking:

$ ./target/release/solana -ul vote-account 3vZMFHQPt618pUnjEnnw5yXbNgEK6PMzLNHEZrroe8Qn
Account Balance: 0.02685864 SOL
Validator Identity: A9ZHywzYR3G17utyPs8kb5Tm7pcrbuaGC56AWuuWLNEp
Authorized Voters: {0: "A9ZHywzYR3G17utyPs8kb5Tm7pcrbuaGC56AWuuWLNEp"}
Authorized Withdrawer: A9ZHywzYR3G17utyPs8kb5Tm7pcrbuaGC56AWuuWLNEp
Credits: 1
Commission: 100%
Root Slot: 0
Recent Timestamp: 2021-02-10T05:49:58Z from slot 31
Recent Votes (using 31/31 entries):
- slot: 31 (confirmation count: 1)
- slot: 30 (confirmation count: 2)
- slot: 29 (confirmation count: 3)
- slot: 28 (confirmation count: 4)
- slot: 27 (confirmation count: 5)
- slot: 26 (confirmation count: 6)
- slot: 25 (confirmation count: 7)
- slot: 24 (confirmation count: 8)
- slot: 23 (confirmation count: 9)
- slot: 22 (confirmation count: 10)
- slot: 21 (confirmation count: 11)
- slot: 20 (confirmation count: 12)
- slot: 19 (confirmation count: 13)
- slot: 18 (confirmation count: 14)
- slot: 17 (confirmation count: 15)
- slot: 16 (confirmation count: 16)
- slot: 15 (confirmation count: 17)
- slot: 14 (confirmation count: 18)
- slot: 13 (confirmation count: 19)
- slot: 12 (confirmation count: 20)
- slot: 11 (confirmation count: 21)
- slot: 10 (confirmation count: 22)
- slot: 9 (confirmation count: 23)
- slot: 8 (confirmation count: 24)
- slot: 7 (confirmation count: 25)
- slot: 6 (confirmation count: 26)
- slot: 5 (confirmation count: 27)
- slot: 4 (confirmation count: 28)
- slot: 3 (confirmation count: 29)
- slot: 2 (confirmation count: 30)
- slot: 1 (confirmation count: 31)
- ... (truncated older 1 votes, which was rooted and became credits)
All Epoch Voting History (using 1/64 entries):
* missed credits include slots unavailable to vote on due to delinquent leaders
- epoch: 0
  credits range: (0..1]
  credits/slots: 1/128

entry.prev_credits, entry.credits
)?;
writeln!(
Expand All @@ -637,6 +675,22 @@ fn show_votes_and_credits(
entry.credits_earned, entry.slots_in_epoch
)?;
}
if let Some(oldest) = epoch_voting_history.iter().next() {
if oldest.prev_credits > 0 {
// Oldest entry doesn't start with 0. so history must be truncated...

// count of this combined pseudo credits range: (0..=oldest.prev_credits] like the above
// (or this is just [1..=oldest.prev_credits] for human's simpler minds)
let count = oldest.prev_credits;

writeln!(
f,
"- ... (omitting {} past rooted votes, which have already been credited)",
count
)?;
}
}

Ok(())
}

Expand Down
2 changes: 1 addition & 1 deletion programs/vote/src/vote_state/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ pub const MAX_LOCKOUT_HISTORY: usize = 31;
pub const INITIAL_LOCKOUT: usize = 2;

// Maximum number of credits history to keep around
const MAX_EPOCH_CREDITS_HISTORY: usize = 64;
pub const MAX_EPOCH_CREDITS_HISTORY: usize = 64;
Copy link
Copy Markdown
Contributor Author

@ryoqun ryoqun Feb 10, 2021

Choose a reason for hiding this comment

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

Making this const pub shouldn't create much burden for us for maintainability or compatibility as this is very sensitive to the abi. So, we won't change this in near future.


#[frozen_abi(digest = "Ch2vVEwos2EjAVqSHCyJjnN2MNX1yrpapZTGhMSCjWUH")]
#[derive(Serialize, Default, Deserialize, Debug, PartialEq, Eq, Clone, AbiExample)]
Expand Down