banking_stage: calculate stake by vote account instead of node pubkey#3049
banking_stage: calculate stake by vote account instead of node pubkey#3049AshwinSekar merged 4 commits intoanza-xyz:masterfrom
Conversation
AshwinSekar
force-pushed
the
luv-vote-pubkey
branch
from
60b1b25 to
e33eaf7
Compare
| let epoch_stakes = self.cached_epoch_stakes.read().unwrap(); | ||
| votes.filter(move |vote| { | ||
| let stake = staked_nodes.get(&vote.pubkey()).copied().unwrap_or(0); | ||
| let stake = epoch_stakes.vote_account_stake(&vote.vote_pubkey()); |
There was a problem hiding this comment.
This now matches fork choice:
| Ok(vote_state_update_instruction) | ||
| if instruction_filter(&vote_state_update_instruction) => | ||
| { | ||
| let &pubkey = message |
There was a problem hiding this comment.
Starry pointed out that this method of identifying the voter is brittle, as it relies on the node id being the first signer.
Grabbing the vote pubkey instead should be safe, as it is required to be the first account on the instruction:
agave/sdk/program/src/vote/instruction.rs
Lines 153 to 154 in 1805924
AshwinSekar
force-pushed
the
luv-vote-pubkey
branch
from
e33eaf7 to
eb6dc10
Compare
AshwinSekar
force-pushed
the
luv-vote-pubkey
branch
from
eb6dc10 to
5b78efb
Compare
| cached_staked_nodes: RwLock::new(bank.current_epoch_staked_nodes().clone()), | ||
| latest_votes_per_pubkey: RwLock::new(HashMap::default()), | ||
| num_unprocessed_votes: AtomicUsize::new(0), | ||
| cached_epoch_stakes: RwLock::new(bank.current_epoch_stakes().clone()), |
There was a problem hiding this comment.
just a note here; cloning is "cheap" since EpochStakes holds 3 Arcs and a u64 internally.
this won't clone the relatively large underlying data
AshwinSekar
force-pushed
the
luv-vote-pubkey
branch
from
5b78efb to
3f1fcd6
Compare
AshwinSekar
force-pushed
the
luv-vote-pubkey
branch
from
3f1fcd6 to
8171af5
Compare
jstarry
left a comment
jstarry
left a comment
There was a problem hiding this comment.
still some places where latest_votes_per_pubkey is used but I was more concerned about the field being named explicitly so I'm happy
I can't unsee it now 😅 , renamed the remaining occurrences |
AshwinSekar
force-pushed
the
luv-vote-pubkey
branch
from
245080e to
eb5cb40
Compare
…anza-xyz#3049) * banking_stage: calculate stake by vote account instead of node pubkey * pr feedback: don't hold epoch stakes write lock for eviction * pr feedback: pubkey -> vote_pubkey renames * pr feedback: rename missed pubkey -> vote_pubkey
3 participants
Problem
latest_unprocessed_votesuses the bank'sstaked_nodesin order to calculate the voter's stake. This structure is based off of thenode_pubkeyto account for validators with multiple vote accounts.Fork choice and other consensus stake operations use
EpochStakesinstead, which calculates stake per vote account.Summary of Changes
Use the
vote_pubkeyinstead, and calculate stake based on thevote_account.