witnet-rust
witnet-rust copied to clipboard
Radon Mode operator only works correctly up to 127 equal elements
Because it uses a i8
counter internally, so it overflows on more than 127 equal elements:
https://github.com/witnet/witnet-rust/blob/c3b1d86ca229ebf8fe5651bfb35d8db9c902eb54/rad/src/reducers/mode.rs#L10
This affects all radon scripts, as well as the tally validation logic. In practice I believe this has not caused any issues yet because a data request with more than 127 witnesses is near the block weight limit, maybe even too large. The fix should be applied in a controlled manner to not cause any accidental hard forks, for example using TAPI with the next protocol upgrade, or a separate WIP.
These tests should pass:
// rad/src/reducers/mode.rs
#[test]
fn test_operate_reduce_mode_128_equal_elements() {
// reduce::mode uses i8 to count frequency, so it only works when there are at most
// 127 equal elements
// Actually, this test passes if compiled in release mode
let array1 = RadonArray::from(vec![RadonInteger::from(1i128).into()]);
let input = RadonArray::from(vec![array1.clone().into(); 128]);
let expected = RadonTypes::from(array1);
let output = mode(&input).unwrap();
assert_eq!(output, expected);
}
#[test]
fn test_operate_reduce_mode_128_equal_elements_and_1_different() {
// reduce::mode uses i8 to count frequency, so it only works when there are at most
// 127 equal elements
// When compiled in debug mode this panics, and in release mode it returns "2".
let array1 = RadonArray::from(vec![RadonInteger::from(1i128).into()]);
let mut input_vec = vec![array1.clone().into(); 128];
input_vec.push(RadonArray::from(vec![RadonInteger::from(2i128).into()]).into());
let input = RadonArray::from(input_vec);
let expected = RadonTypes::from(array1);
let output = mode(&input).unwrap();
assert_eq!(output, expected);
}
This is the output of the second test in release mode:
---- reducers::mode::tests::test_operate_reduce_mode_128_equal_elements_and_1_different stdout ----
thread 'reducers::mode::tests::test_operate_reduce_mode_128_equal_elements_and_1_different' panicked at 'assertion failed: `(left == right)`
left: `Array(RadonArray { value: [Integer(RadonInteger { value: 2 })], is_homogeneous: true })`,
right: `Array(RadonArray { value: [Integer(RadonInteger { value: 1 })], is_homogeneous: true })`', rad/src/reducers/mode.rs:208:9
Nice catch! :kudos: