-
Notifications
You must be signed in to change notification settings - Fork 745
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add
safe_sum
and use it in state_processing (#1620)
## Issue Addressed Closes #1098 ## Proposed Changes Add a `SafeArithIter` trait with a `safe_sum` method, and use it in `state_processing`. This seems to be the only place in `consensus` where it is relevant -- i.e. where we were using `sum` and the integer_arith lint is enabled. ## Additional Info This PR doesn't include any Clippy linting to prevent `sum` from being called. It seems there is no existing Clippy lint that suits our purpose, but I'm going to look into that and maybe schedule writing one as a lower-priority task. This theoretically _is_ a consensus breaking change, but it shouldn't impact Medalla (or any other testnet) because `slashings` shouldn't overflow!
- Loading branch information
1 parent
4fca306
commit 7aceff4
Showing
3 changed files
with
76 additions
and
3 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,70 @@ | ||
use crate::{Result, SafeArith}; | ||
|
||
/// Extension trait for iterators, providing a safe replacement for `sum`. | ||
pub trait SafeArithIter<T> { | ||
fn safe_sum(self) -> Result<T>; | ||
} | ||
|
||
impl<I, T> SafeArithIter<T> for I | ||
where | ||
I: Iterator<Item = T> + Sized, | ||
T: SafeArith, | ||
{ | ||
fn safe_sum(mut self) -> Result<T> { | ||
self.try_fold(T::ZERO, |acc, x| acc.safe_add(x)) | ||
} | ||
} | ||
|
||
#[cfg(test)] | ||
mod test { | ||
use super::*; | ||
use crate::ArithError; | ||
|
||
#[test] | ||
fn empty_sum() { | ||
let v: Vec<u64> = vec![]; | ||
assert_eq!(v.into_iter().safe_sum(), Ok(0)); | ||
} | ||
|
||
#[test] | ||
fn unsigned_sum_small() { | ||
let v = vec![400u64, 401, 402, 403, 404, 405, 406]; | ||
assert_eq!( | ||
v.iter().copied().safe_sum().unwrap(), | ||
v.iter().copied().sum() | ||
); | ||
} | ||
|
||
#[test] | ||
fn unsigned_sum_overflow() { | ||
let v = vec![u64::MAX, 1]; | ||
assert_eq!(v.into_iter().safe_sum(), Err(ArithError::Overflow)); | ||
} | ||
|
||
#[test] | ||
fn signed_sum_small() { | ||
let v = vec![-1i64, -2i64, -3i64, 3, 2, 1]; | ||
assert_eq!(v.into_iter().safe_sum(), Ok(0)); | ||
} | ||
|
||
#[test] | ||
fn signed_sum_overflow_above() { | ||
let v = vec![1, 2, 3, 4, i16::MAX, 0, 1, 2, 3]; | ||
assert_eq!(v.into_iter().safe_sum(), Err(ArithError::Overflow)); | ||
} | ||
|
||
#[test] | ||
fn signed_sum_overflow_below() { | ||
let v = vec![i16::MIN, -1]; | ||
assert_eq!(v.into_iter().safe_sum(), Err(ArithError::Overflow)); | ||
} | ||
|
||
#[test] | ||
fn signed_sum_almost_overflow() { | ||
let v = vec![i64::MIN, 1, -1i64, i64::MAX, i64::MAX, 1]; | ||
assert_eq!( | ||
v.iter().copied().safe_sum().unwrap(), | ||
v.iter().copied().sum() | ||
); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters