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

Insert missing entries in CopyVoteAccount #37

Merged
merged 11 commits into from
May 27, 2024
Merged

Conversation

ebatsell
Copy link
Collaborator

Problem:
Due to unreliability in landing transactions in epoch 598 (when chain was super congested), about half of active validators did not get a single ValidatorHistoryEntry created, so their epoch credits were never copied over in future epochs. This is bad for Stakenet scoring, and this issue could happen in the future if congestion gets bad again.

Solution:
In the CopyVoteAccount instruction, when epochs are detected in the vote account that don't exist in the CircBuf, insert those new entries and shift all entries with greater epochs forward by one. This will evict the oldest entries if we are wrapping around. After that, epoch credits are copied to the old entries. We will then be able to retroactively call:
CopyTipDistributionAccount and UpdateStakeHistory on those validators.

Notes:

  • We are still within the 64 epoch vote credit range, so epoch credits from 598 and any other randomly skipped entries will get added
  • In the most extreme case possible of 64 entries inserted (max epochs in vote account), this instruction uses ~270k CUs. Most of the time none will be added, and when this first runs, a handful will be added for some validators. Normal CU usage is <30k.

@ebatsell ebatsell requested review from buffalu and coachchucksol May 23, 2024 02:19
Copy link
Contributor

@CoachChuckFF CoachChuckFF left a comment

Choose a reason for hiding this comment

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

Everything looks good!

Two suggestions:

  1. Consider making get_max_epoch and get_min_epoch as helper functions in the Circle Buf
  2. Potentially add another integration test(s) in test_vote_account to check for wraparounds

/// Returns None if either start_epoch or end_epoch is not in the CircBuf
/// Given a new entry and epoch, inserts the entry into the buffer in sorted order
/// Will not insert if the epoch is out of range or already exists in the buffer
fn insert(&mut self, entry: ValidatorHistoryEntry, epoch: u16) -> Result<()> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

rip, not doing index = epoch % self.history.len() might have been a mistake

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

💯

@ebatsell ebatsell merged commit d85e607 into master May 27, 2024
6 checks passed
@ebatsell ebatsell deleted the evan/insert-missing branch May 27, 2024 17:15
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.

3 participants