diff --git a/mina-p2p-messages/src/number.rs b/mina-p2p-messages/src/number.rs index 221589e144..1cf67004cd 100644 --- a/mina-p2p-messages/src/number.rs +++ b/mina-p2p-messages/src/number.rs @@ -197,10 +197,68 @@ macro_rules! binprot_number { binprot_number!(i32, i32); binprot_number!(i64, i64); -binprot_number!(u32, i32); -binprot_number!(u64, i64); binprot_number!(f64, f64); +// Custom implementations for u32 and u64 to handle the full i64 range +// without TryFromIntError. The binprot format stores all integers as i64 +// internally, and conversion can fail if the value is outside the target +// type's range. We read as i64 first, then cast to handle out-of-range values +// gracefully. Writing still uses the smaller type to maintain wire format +// compatibility. +// +// Note: This introduces intentional asymmetry between read and write for the sake +// of robustness. When reading, we accept any i64 value and cast using wrapping +// semantics. When writing, we cast to the signed type (which may change the sign +// bit for large values). This is acceptable because: +// 1. The wire format is defined by the original OCaml implementation which uses +// signed integers for the binprot encoding +// 2. Rust code reading these values will interpret them correctly via the cast +// 3. This prevents connection drops from malformed or unexpected data +impl binprot::BinProtRead for Number { + fn binprot_read(r: &mut R) -> Result + where + Self: Sized, + { + // Read the value as i64 first to avoid TryFromIntError when the value + // is outside i32 range, then cast to u32 using wrapping semantics. + // This handles cases where the binprot stream contains values outside + // the i32 range (e.g., large nonce values). + let value = i64::binprot_read(r)?; + Ok(Self(value as u32)) + } +} + +impl binprot::BinProtWrite for Number { + fn binprot_write(&self, w: &mut W) -> std::io::Result<()> { + // Write as i32 to maintain wire format compatibility with the OCaml + // implementation. For values > i32::MAX, the cast will reinterpret + // the bits as a negative i32, which is the expected behavior. + (self.0 as i32).binprot_write(w) + } +} + +impl binprot::BinProtRead for Number { + fn binprot_read(r: &mut R) -> Result + where + Self: Sized, + { + // Read the value as i64 first to handle the full range without errors, + // then cast to u64 using wrapping semantics. This prevents TryFromIntError + // for negative i64 values or values outside u64::MIN..=i64::MAX. + let value = i64::binprot_read(r)?; + Ok(Self(value as u64)) + } +} + +impl binprot::BinProtWrite for Number { + fn binprot_write(&self, w: &mut W) -> std::io::Result<()> { + // Write as i64 to maintain wire format compatibility. For values > i64::MAX, + // the cast will reinterpret the bits as a negative i64, matching the OCaml + // implementation's behavior. + (self.0 as i64).binprot_write(w) + } +} + #[cfg(test)] mod tests { use binprot::{BinProtRead, BinProtWrite}; @@ -257,4 +315,57 @@ mod tests { max_number_test!(u32_max, u32); max_number_test!(u64_max, u64); + + /// Test that Number can read i64 values outside i32 range without error. + /// This is a regression test for the TryFromIntError issue. + #[test] + fn u32_read_large_i64() { + // Create a binprot-encoded i64 value larger than i32::MAX + let large_i64 = (i32::MAX as i64) + 1000; + let mut buf = Vec::new(); + large_i64.binprot_write(&mut buf).unwrap(); + + // This should not fail with TryFromIntError + let mut r = buf.as_slice(); + let result = super::Number::::binprot_read(&mut r); + assert!(result.is_ok(), "Should handle i64 values outside i32 range"); + + // The value should wrap as expected when cast from i64 to u32 + let n = result.unwrap(); + assert_eq!(n.0, large_i64 as u32); + } + + /// Test that Number can read negative i64 values without error. + #[test] + fn u32_read_negative_i64() { + // Create a binprot-encoded negative i64 value + let negative_i64 = -1000i64; + let mut buf = Vec::new(); + negative_i64.binprot_write(&mut buf).unwrap(); + + // This should not fail with TryFromIntError + let mut r = buf.as_slice(); + let result = super::Number::::binprot_read(&mut r); + assert!(result.is_ok(), "Should handle negative i64 values"); + + // The value should wrap as expected when cast from i64 to u32 + let n = result.unwrap(); + assert_eq!(n.0, negative_i64 as u32); + } + + /// Test that Number can read i64 values without error. + #[test] + fn u64_read_i64() { + // Test with a negative value (which would fail with try_from) + let negative_i64 = -5000i64; + let mut buf = Vec::new(); + negative_i64.binprot_write(&mut buf).unwrap(); + + let mut r = buf.as_slice(); + let result = super::Number::::binprot_read(&mut r); + assert!(result.is_ok(), "Should handle negative i64 values"); + + let n = result.unwrap(); + assert_eq!(n.0, negative_i64 as u64); + } }