Handle set new root on new frozen slots #66
Conversation
core/src/replay_stage.rs
Outdated
| r_bank_forks.get(new_frozen_root).unwrap(), | ||
| ) | ||
| }; | ||
| assert!(new_frozen_root > current_root_slot); |
There was a problem hiding this comment.
👍 finalized have to descend from each other, replay cannot proceed out of order
core/src/replay_stage.rs
Outdated
| (r_bank_forks.ancestors(), r_bank_forks.descendants()) | ||
| }; | ||
| let did_complete_bank = Self::replay_active_banks( | ||
| let completed_slots = Self::replay_active_banks( |
There was a problem hiding this comment.
nit: I think complete is pretty vague here, we really mean "slots_completed_replay"?
There was a problem hiding this comment.
renamed to new_frozen_slots
| let new_frozen_root = completed_slots | ||
| .into_iter() | ||
| .filter(|slot| cert_pool.is_finalized_slot(*slot)) | ||
| .max(); |
There was a problem hiding this comment.
Is it possible that we have:
slot 100 -> 101
101 got finalized cert but somehow the finalization of 100 hasn't fully propagated so it doesn't have a finalization cert?
core/src/replay_stage.rs
Outdated
| ) | ||
| }; | ||
| assert!(new_frozen_root > current_root_slot); | ||
| if let Err(e) = Self::alpenglow_handle_new_root( |
There was a problem hiding this comment.
Should we have two sources of new_alpenglow_root, one in handle_votable_alpenglow_bank and one in here? Should we just have one set_root step at the end of Alpenglow replay loop (so no matter whether your own vote or replayed votes nailed a new cert, we just have one check_and_handle_new_root in the loop?)
There was a problem hiding this comment.
yup that makes sense, there will be a third one as well once we add ingesting votes from gossip. I moved it out into the main replay loop, also was good to cut down on a lot of the arguments passed to the alpenglow vote functions 😃
3 participants
Problem
Slots can get a finalization certificate before we freeze them
Summary of Changes
On bank freeze, check if we have the finalization certificate
Fixes #