Add local cluster utitlity functions#355
Conversation
carllin
force-pushed
the
FixLocalClusterUtility
branch
from
9508ae0 to
f6b4f6c
Compare
carllin
force-pushed
the
FixLocalClusterUtility
branch
from
f6b4f6c to
0026ec6
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #355 +/- ##
=========================================
- Coverage 81.9% 81.9% -0.1%
=========================================
Files 838 838
Lines 227366 227366
=========================================
- Hits 186292 186291 -1
- Misses 41074 41075 +1 |
| self.last_vote = new_vote; | ||
| } | ||
|
|
||
| fn record_bank_vote_and_update_lockouts( |
There was a problem hiding this comment.
nit: can we reuse record_vote instead of changing the visibility on this? looks like that one is already used for tests.
| let client = ThinClient::new(rpc, tpu, connection_cache.clone()); | ||
| loop { | ||
| let root_slot = client | ||
| .get_slot_with_commitment(CommitmentConfig::finalized()) |
There was a problem hiding this comment.
don't know the context of where you plan to use this yet, but is this more preferable than checking root from VoteState?
There was a problem hiding this comment.
That was the other option, but this was intended as a faster replacement for check_for_new_roots() which goes over the client.
Directly introspecting the tower has always been more of a hack, since LocalCluster was always supposed to support a generic interface for interacting with any cluster, even one that's remote. So when possible, trying to avoid it
| } | ||
| } | ||
|
|
||
| pub fn simulate_tower(node_keypair: &Keypair, votes: Vec<(Slot, Hash)>, tower_path: PathBuf) { |
There was a problem hiding this comment.
nit: simulate_tower is vague, this is really building new tower from votes?
There was a problem hiding this comment.
It's applying new votes to an existing tower, so maybe apply_votes_to_tower(). changed.
| } | ||
|
|
||
| pub fn check_min_slot_is_rooted( | ||
| min_slot: Slot, |
There was a problem hiding this comment.
I'm confused: why is this min_slot instead of just slot? (How is min_slot different from a normal slot?)
There was a problem hiding this comment.
it's not guaranteed that at the time you check that the root is exactly some slot X, the validator could have made another root. Thus we just check for a minimum
| pub fn check_min_slot_is_rooted( | ||
| min_slot: Slot, | ||
| contact_infos: &[ContactInfo], | ||
| connection_cache: &Arc<ConnectionCache>, |
There was a problem hiding this comment.
nit: Probably better to describe in function name or comments we are testing the designated slot is root on all given contact_infos?
There was a problem hiding this comment.
it's not that we are checking that min_slot is rooted, we are checking that some slot greater than the min_slot is rooted
| for ingress_node in contact_infos.iter() { | ||
| let (rpc, tpu) = LegacyContactInfo::try_from(ingress_node) | ||
| .map(|node| get_client_facing_addr(connection_cache.protocol(), node)) | ||
| .unwrap(); |
There was a problem hiding this comment.
Are we sure this unwrap() will always succeed?
There was a problem hiding this comment.
yup if the contact info is well formed seems like it should always work
| } | ||
| } | ||
|
|
||
| #[cfg(test)] |
There was a problem hiding this comment.
think you can use the new guy here:
#[cfg(feature = "dev-context-only-utils")]
There was a problem hiding this comment.
didn't even realize this existed, works much wao
There was a problem hiding this comment.
huh works locally but is failing CI: https://buildkite.com/anza/agave/builds/1225#018e68a5-2942-41ba-973a-bf89c03c8950
There was a problem hiding this comment.
huh strange. I'm fine if you push it without the flag for now
carllin
force-pushed
the
FixLocalClusterUtility
branch
from
14231d2 to
c5352cc
Compare
I need to perform this kind of updates quite frequently. It is good to have the process documented and the commonly used command written done for the purposes of copy and paste :)
4 participants
Problem
Missing some useful utility function
Summary of Changes
Fixes #