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

Update is_locked_out cache when adopting on chain vote state #33341

Merged
merged 3 commits into from
Sep 25, 2023

Conversation

AshwinSekar
Copy link
Contributor

Problem

The is_locked_out cache is not updated when we adopt the on chain vote state.
This can cause a similar situation to #32880 (detailed in #32944) because we attempt to vote on a block we are locked out on which will fail.

Summary of Changes

  • Update the cache
  • Don't update last vote when there is a vote program error

Fixes #

@AshwinSekar AshwinSekar requested a review from carllin September 21, 2023 05:25
core/src/consensus.rs Outdated Show resolved Hide resolved
@AshwinSekar AshwinSekar force-pushed the tower-sync-last-vote branch 2 times, most recently from e167aca to 9558abd Compare September 21, 2023 05:47
@codecov
Copy link

codecov bot commented Sep 21, 2023

Codecov Report

Merging #33341 (8f00433) into master (997aa0a) will increase coverage by 0.0%.
Report is 4 commits behind head on master.
The diff coverage is 98.5%.

@@           Coverage Diff           @@
##           master   #33341   +/-   ##
=======================================
  Coverage    81.9%    81.9%           
=======================================
  Files         798      798           
  Lines      216579   216642   +63     
=======================================
+ Hits       177481   177572   +91     
+ Misses      39098    39070   -28     

@AshwinSekar AshwinSekar added v1.14 v1.16 PRs that should be backported to v1.16 labels Sep 22, 2023
Comment on lines 3177 to 3186
for slot in frozen_banks.iter().map(|b| b.slot()) {
let stats = progress
.get_fork_stats_mut(slot)
.expect("All frozen banks must exist in the Progress map");
stats.is_locked_out = tower.is_locked_out(
slot,
ancestors
.get(&slot)
.expect("Ancestors map should contain slot for is_locked_out() check"),
);
Copy link
Contributor

Choose a reason for hiding this comment

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

this is a good catch!

Don't all of these tower based checks

stats.vote_threshold =
tower.check_vote_stake_threshold(bank_slot, &stats.voted_stakes, stats.total_stake);
stats.is_locked_out = tower.is_locked_out(
bank_slot,
ancestors
.get(&bank_slot)
.expect("Ancestors map should contain slot for is_locked_out() check"),
);
stats.has_voted = tower.has_voted(bank_slot);
stats.is_recent = tower.is_recent(bank_slot);
need to be redone?

Might be easier to move all those tower-based checks into a separate function that we can call from both places so that we don't miss this in the future for any added checks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch, added all of these as well

@AshwinSekar AshwinSekar merged commit 85cc6ac into solana-labs:master Sep 25, 2023
mergify bot pushed a commit that referenced this pull request Sep 25, 2023
* Update is_locked_out cache when adopting on chain vote state

* extend to all cached tower checks

* upgrade error to panic

(cherry picked from commit 85cc6ac)

# Conflicts:
#	core/src/consensus.rs
mergify bot pushed a commit that referenced this pull request Sep 25, 2023
* Update is_locked_out cache when adopting on chain vote state

* extend to all cached tower checks

* upgrade error to panic

(cherry picked from commit 85cc6ac)

# Conflicts:
#	core/src/consensus.rs
AshwinSekar added a commit that referenced this pull request Sep 26, 2023
…backport of #33341) (#33402)

* Update is_locked_out cache when adopting on chain vote state (#33341)

* Update is_locked_out cache when adopting on chain vote state

* extend to all cached tower checks

* upgrade error to panic

(cherry picked from commit 85cc6ac)

# Conflicts:
#	core/src/consensus.rs

* Fix conflicts

---------

Co-authored-by: Ashwin Sekar <[email protected]>
AshwinSekar added a commit that referenced this pull request Sep 26, 2023
…backport of #33341) (#33403)

* Update is_locked_out cache when adopting on chain vote state (#33341)

* Update is_locked_out cache when adopting on chain vote state

* extend to all cached tower checks

* upgrade error to panic

(cherry picked from commit 85cc6ac)

# Conflicts:
#	core/src/consensus.rs

* Fix conflicts

---------

Co-authored-by: Ashwin Sekar <[email protected]>
bw-solana pushed a commit to bw-solana/solana that referenced this pull request Jan 10, 2025
…backport of solana-labs#33341) (solana-labs#33402)

* Update is_locked_out cache when adopting on chain vote state (solana-labs#33341)

* Update is_locked_out cache when adopting on chain vote state

* extend to all cached tower checks

* upgrade error to panic

(cherry picked from commit 85cc6ac)

# Conflicts:
#	core/src/consensus.rs

* Fix conflicts

---------

Co-authored-by: Ashwin Sekar <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
v1.16 PRs that should be backported to v1.16
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants