From a4da2fbde6098f7114809876db6b715a346b66dc Mon Sep 17 00:00:00 2001 From: Evan B Date: Tue, 16 Jan 2024 10:41:14 -0500 Subject: [PATCH] Tweak epoch_range response (#17) Tweaks epoch_range response so it's easier to align the output from different objects and guarantee that the epochs will be aligned --- programs/validator-history/src/state.rs | 151 +++++++++++++++++++----- 1 file changed, 124 insertions(+), 27 deletions(-) diff --git a/programs/validator-history/src/state.rs b/programs/validator-history/src/state.rs index 00592137..e08d2f36 100644 --- a/programs/validator-history/src/state.rs +++ b/programs/validator-history/src/state.rs @@ -137,12 +137,11 @@ macro_rules! field_range { let epoch_range = $self.epoch_range($start_epoch, $end_epoch); epoch_range .iter() - .map(|entry| { - if entry.$field != ValidatorHistoryEntry::default().$field { - Some(entry.$field) - } else { - None - } + .map(|maybe_entry| { + maybe_entry + .as_ref() + .map(|entry| entry.$field) + .filter(|&field| field != ValidatorHistoryEntry::default().$field) }) .collect::>>() }}; @@ -181,12 +180,27 @@ impl CircBuf { /// Returns &ValidatorHistoryEntry for each existing entry in range [start_epoch, end_epoch], factoring for wraparound /// Returns None if either start_epoch or end_epoch is not in the CircBuf - pub fn epoch_range(&self, start_epoch: u16, end_epoch: u16) -> Vec<&ValidatorHistoryEntry> { + pub fn epoch_range( + &self, + start_epoch: u16, + end_epoch: u16, + ) -> Vec> { // creates an iterator that lays out the entries in consecutive order, handling wraparound - self.arr[(self.idx as usize + 1)..] // if self.idx + 1 == self.arr.len() this will just return an empty slice + let mut entries = self.arr[(self.idx as usize + 1)..] // if self.idx + 1 == self.arr.len() this will just return an empty slice .iter() .chain(self.arr[..=(self.idx as usize)].iter()) .filter(|entry| entry.epoch >= start_epoch && entry.epoch <= end_epoch) + .peekable(); + (start_epoch..=end_epoch) + .map(|epoch| { + if let Some(&entry) = entries.peek() { + if entry.epoch == epoch { + entries.next(); + return Some(entry); + } + } + None + }) .collect() } @@ -559,6 +573,18 @@ pub struct CircBufCluster { pub arr: [ClusterHistoryEntry; MAX_ITEMS], } +#[cfg(test)] +impl Default for CircBufCluster { + fn default() -> Self { + Self { + arr: [ClusterHistoryEntry::default(); MAX_ITEMS], + idx: 0, + is_empty: 1, + padding: [0; 7], + } + } +} + impl CircBufCluster { pub fn push(&mut self, item: ClusterHistoryEntry) { self.idx = (self.idx + 1) % self.arr.len() as u64; @@ -592,12 +618,27 @@ impl CircBufCluster { /// Returns &ClusterHistoryEntry for each existing entry in range [start_epoch, end_epoch], factoring for wraparound /// Returns None if either start_epoch or end_epoch is not in the CircBuf - pub fn epoch_range(&self, start_epoch: u16, end_epoch: u16) -> Vec<&ClusterHistoryEntry> { + pub fn epoch_range( + &self, + start_epoch: u16, + end_epoch: u16, + ) -> Vec> { // creates an iterator that lays out the entries in consecutive order, handling wraparound - self.arr[(self.idx as usize + 1)..] + let mut entries = self.arr[(self.idx as usize + 1)..] // if self.idx + 1 == self.arr.len() this will just return an empty slice .iter() .chain(self.arr[..=(self.idx as usize)].iter()) .filter(|entry| entry.epoch >= start_epoch && entry.epoch <= end_epoch) + .peekable(); + (start_epoch..=end_epoch) + .map(|epoch| { + if let Some(&entry) = entries.peek() { + if entry.epoch == epoch { + entries.next(); + return Some(entry); + } + } + None + }) .collect() } @@ -617,12 +658,11 @@ impl CircBufCluster { let epoch_range = self.epoch_range(start_epoch, end_epoch); epoch_range .iter() - .map(|entry| { - if entry.total_blocks != ClusterHistoryEntry::default().total_blocks { - Some(entry.total_blocks) - } else { - None - } + .map(|maybe_entry| { + maybe_entry + .as_ref() + .map(|entry| entry.total_blocks) + .filter(|&field| field != ClusterHistoryEntry::default().total_blocks) }) .collect::>>() } @@ -674,9 +714,12 @@ mod tests { circ_buf.push(entry); } // Test epoch range [0, 3] - let epoch_range: Vec<&ValidatorHistoryEntry> = circ_buf.epoch_range(0, 3); + let epoch_range: Vec> = circ_buf.epoch_range(0, 3); assert_eq!( - epoch_range.iter().map(|e| e.epoch).collect::>(), + epoch_range + .iter() + .filter_map(|maybe_e| maybe_e.map(|e| e.epoch)) + .collect::>(), vec![0, 1, 2, 3] ); @@ -696,29 +739,41 @@ mod tests { // Test epoch range [0, 3] let epoch_range = circ_buf.epoch_range(0, 3); assert_eq!( - epoch_range.iter().map(|e| e.epoch).collect::>(), - vec![0, 1, 3] + epoch_range + .iter() + .map(|maybe_e| maybe_e.map(|e| e.epoch)) + .collect::>>(), + vec![Some(0), Some(1), None, Some(3)] ); // same start and end epoch // Test end epoch out of range let epoch_range = circ_buf.epoch_range(0, 5); assert_eq!( - epoch_range.iter().map(|e| e.epoch).collect::>(), - vec![0, 1, 3] + epoch_range + .iter() + .map(|maybe_e| maybe_e.map(|e| e.epoch)) + .collect::>>(), + vec![Some(0), Some(1), None, Some(3), None, None] ); // None if start epoch is none let epoch_range = circ_buf.epoch_range(2, 3); assert_eq!( - epoch_range.iter().map(|e| e.epoch).collect::>(), - vec![3] + epoch_range + .iter() + .map(|maybe_e| maybe_e.map(|e| e.epoch)) + .collect::>>(), + vec![None, Some(3)] ); let epoch_range = circ_buf.epoch_range(3, 3); assert_eq!( - epoch_range.iter().map(|e| e.epoch).collect::>(), - vec![3] + epoch_range + .iter() + .map(|maybe_e| maybe_e.map(|e| e.epoch)) + .collect::>>(), + vec![Some(3)] ); let epoch_range = circ_buf.epoch_range(4, 3); @@ -736,8 +791,50 @@ mod tests { let epoch_range = circ_buf.epoch_range(circ_buf.arr.len() as u16 - 4, circ_buf.arr.len() as u16 + 4); assert_eq!( - epoch_range.iter().map(|e| e.epoch).collect::>(), + epoch_range + .iter() + .filter_map(|maybe_e| maybe_e.map(|e| e.epoch)) + .collect::>(), + vec![508, 509, 510, 511, 512, 513, 514, 515, 516] + ); + + // Test ClusterHistory CircBuf epoch range with wraparound + let mut cluster_circ_buf = CircBufCluster::default(); + (0..=cluster_circ_buf.arr.len() + 4).for_each(|i| { + cluster_circ_buf.push(ClusterHistoryEntry { + epoch: i as u16, + ..ClusterHistoryEntry::default() + }) + }); + let epoch_range = cluster_circ_buf.epoch_range(508, 516); + assert_eq!( + epoch_range + .iter() + .filter_map(|maybe_e| maybe_e.map(|e| e.epoch)) + .collect::>(), vec![508, 509, 510, 511, 512, 513, 514, 515, 516] ); + + cluster_circ_buf = CircBufCluster::default(); + for i in 0..4 { + if i == 2 { + continue; + } + let entry = ClusterHistoryEntry { + epoch: i, + ..ClusterHistoryEntry::default() + }; + cluster_circ_buf.push(entry); + } + + // Test with None epoch + let epoch_range = cluster_circ_buf.epoch_range(0, 3); + assert_eq!( + epoch_range + .iter() + .map(|maybe_e| maybe_e.map(|e| e.epoch)) + .collect::>>(), + vec![Some(0), Some(1), None, Some(3)] + ); } }