diff --git a/zaino-state/src/chain_index/types/db/legacy.rs b/zaino-state/src/chain_index/types/db/legacy.rs index 3ee7627ee..7942d50d8 100644 --- a/zaino-state/src/chain_index/types/db/legacy.rs +++ b/zaino-state/src/chain_index/types/db/legacy.rs @@ -282,118 +282,8 @@ impl FixedEncodedLen for TransactionHash { const ENCODED_LEN: usize = 32; } -/// Block height. -/// -/// NOTE: Encoded as 4-byte big-endian byte-string to ensure height ordering -/// for keys in Lexicographically sorted B-Tree. -#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] -#[cfg_attr(test, derive(serde::Serialize, serde::Deserialize))] -pub struct Height(pub(crate) u32); - -/// The first block -pub const GENESIS_HEIGHT: Height = Height(0); - -impl TryFrom for Height { - type Error = &'static str; - - fn try_from(height: u32) -> Result { - // Zebra enforces Height <= 2^31 - 1 - if height <= zebra_chain::block::Height::MAX.0 { - Ok(Self(height)) - } else { - Err("height must be ≤ 2^31 - 1") - } - } -} - -impl From for u32 { - fn from(h: Height) -> Self { - h.0 - } -} - -impl std::ops::Add for Height { - type Output = Self; - - fn add(self, rhs: u32) -> Self::Output { - Height(self.0 + rhs) - } -} - -impl std::ops::Sub for Height { - type Output = Self; - - fn sub(self, rhs: u32) -> Self::Output { - Height(self.0 - rhs) - } -} - -impl std::fmt::Display for Height { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", self.0) - } -} - -impl std::str::FromStr for Height { - type Err = &'static str; - - fn from_str(s: &str) -> Result { - let h = s.parse::().map_err(|_| "invalid u32")?; - Self::try_from(h) - } -} - -impl From for zebra_chain::block::Height { - fn from(h: Height) -> Self { - zebra_chain::block::Height(h.0) - } -} - -impl TryFrom for Height { - type Error = &'static str; - - fn try_from(h: zebra_chain::block::Height) -> Result { - Height::try_from(h.0) - } -} - -impl From for zcash_protocol::consensus::BlockHeight { - fn from(h: Height) -> Self { - zcash_protocol::consensus::BlockHeight::from(h.0) - } -} - -impl TryFrom for Height { - type Error = &'static str; - - fn try_from(h: zcash_protocol::consensus::BlockHeight) -> Result { - Height::try_from(u32::from(h)) - } -} - -impl ZainoVersionedSerialise for Height { - const VERSION: u8 = version::V1; - - fn encode_body(&self, w: &mut W) -> io::Result<()> { - // Height must sort lexicographically - write **big-endian** - write_u32_be(w, self.0) - } - - fn decode_latest(r: &mut R) -> io::Result { - Self::decode_v1(r) - } - - fn decode_v1(r: &mut R) -> io::Result { - let raw = read_u32_be(r)?; - Height::try_from(raw).map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e)) - } -} - -/// Height = 4-byte big-endian body. -impl FixedEncodedLen for Height { - /// 4 bytes, BE - const ENCODED_LEN: usize = 4; -} +// Height type has been moved to primitives/height.rs +pub use super::primitives::{Height, GENESIS_HEIGHT}; /// Numerical index of subtree / shard roots. /// diff --git a/zaino-state/src/chain_index/types/db/primitives.rs b/zaino-state/src/chain_index/types/db/primitives.rs index f40bf037a..36d1f9ffb 100644 --- a/zaino-state/src/chain_index/types/db/primitives.rs +++ b/zaino-state/src/chain_index/types/db/primitives.rs @@ -5,3 +5,7 @@ //! - ShardIndex //! - ScriptType //! - ShardRoot + +mod height; + +pub use height::{Height, GENESIS_HEIGHT}; diff --git a/zaino-state/src/chain_index/types/db/primitives/height.rs b/zaino-state/src/chain_index/types/db/primitives/height.rs new file mode 100644 index 000000000..12d483fa8 --- /dev/null +++ b/zaino-state/src/chain_index/types/db/primitives/height.rs @@ -0,0 +1,210 @@ +//! Block height type and utilities. + +use core2::io::{self, Read, Write}; + +use crate::chain_index::encoding::{ + read_u32_be, version, write_u32_be, FixedEncodedLen, ZainoVersionedSerialise, +}; + +/// Block height. +/// +/// NOTE: Encoded as 4-byte big-endian byte-string to ensure height ordering +/// for keys in Lexicographically sorted B-Tree. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] +#[cfg_attr(test, derive(serde::Serialize, serde::Deserialize))] +pub struct Height(pub(crate) u32); + +/// The first block +pub const GENESIS_HEIGHT: Height = Height(0); + +impl TryFrom for Height { + type Error = &'static str; + + fn try_from(height: u32) -> Result { + // Zebra enforces Height <= 2^31 - 1 + if height <= zebra_chain::block::Height::MAX.0 { + Ok(Self(height)) + } else { + Err("height must be ≤ 2^31 - 1") + } + } +} + +impl From for u32 { + fn from(h: Height) -> Self { + h.0 + } +} + +impl std::ops::Add for Height { + type Output = Self; + + fn add(self, rhs: u32) -> Self::Output { + Height(self.0 + rhs) + } +} + +impl std::ops::Sub for Height { + type Output = Self; + + fn sub(self, rhs: u32) -> Self::Output { + Height(self.0 - rhs) + } +} + +impl std::fmt::Display for Height { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.0) + } +} + +impl std::str::FromStr for Height { + type Err = &'static str; + + fn from_str(s: &str) -> Result { + let h = s.parse::().map_err(|_| "invalid u32")?; + Self::try_from(h) + } +} + +impl From for zebra_chain::block::Height { + fn from(h: Height) -> Self { + zebra_chain::block::Height(h.0) + } +} + +impl TryFrom for Height { + type Error = &'static str; + + fn try_from(h: zebra_chain::block::Height) -> Result { + Height::try_from(h.0) + } +} + +impl From for zcash_protocol::consensus::BlockHeight { + fn from(h: Height) -> Self { + zcash_protocol::consensus::BlockHeight::from(h.0) + } +} + +impl TryFrom for Height { + type Error = &'static str; + + fn try_from(h: zcash_protocol::consensus::BlockHeight) -> Result { + Height::try_from(u32::from(h)) + } +} + +impl ZainoVersionedSerialise for Height { + const VERSION: u8 = version::V1; + + fn encode_body(&self, w: &mut W) -> io::Result<()> { + // Height must sort lexicographically - write **big-endian** + write_u32_be(w, self.0) + } + + fn decode_latest(r: &mut R) -> io::Result { + Self::decode_v1(r) + } + + fn decode_v1(r: &mut R) -> io::Result { + let raw = read_u32_be(r)?; + Height::try_from(raw).map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e)) + } +} + +/// Height = 4-byte big-endian body. +impl FixedEncodedLen for Height { + /// 4 bytes, BE + const ENCODED_LEN: usize = 4; +} + +#[cfg(test)] +mod height_safety_tests { + use super::*; + + /// This test demonstrates the underflow bug in Height subtraction. + /// When subtracting from genesis (Height(0)), the result wraps to u32::MAX + /// in release mode, or panics in debug mode. + /// + /// Real-world impact: This breaks reorg detection at genesis in non_finalised_state.rs:390 + /// where the code does `let mut next_height_down = best_tip.height - 1;` without checking + /// if height is 0. The wrapped value causes silent failure instead of ReorgError::AtGenesis. + #[test] + #[cfg(not(debug_assertions))] // Only runs in release mode + fn test_underflow_wraps_to_max_in_release() { + let genesis = GENESIS_HEIGHT; // Height(0) + + // In release mode, this wraps to u32::MAX instead of detecting the error + let result = genesis - 1; + + // This assertion proves the bug: subtracting from 0 gives 4,294,967,295! + assert_eq!(result.0, u32::MAX); + assert_eq!(result.0, 4_294_967_295); + } + + /// This test demonstrates the underflow bug panics in debug mode. + /// Uncommenting this test in debug builds will cause a panic. + #[test] + #[should_panic] + #[cfg(debug_assertions)] // Only runs in debug mode + fn test_underflow_panics_in_debug() { + let genesis = GENESIS_HEIGHT; // Height(0) + + // In debug mode, this panics with "attempt to subtract with overflow" + let _result = genesis - 1; + } + + /// This test demonstrates the overflow bug in Height addition. + /// When adding to a height near u32::MAX, the result wraps around + /// in release mode, or panics in debug mode. + #[test] + #[cfg(not(debug_assertions))] // Only runs in release mode + fn test_overflow_wraps_in_release() { + let high_height = Height(u32::MAX - 10); + + // Adding 20 to (u32::MAX - 10) should overflow + // In release mode, this wraps around + let result = high_height + 20; + + // Proves wraparound: (u32::MAX - 10) + 20 = 9 + assert_eq!(result.0, 9); + + // This could happen if: + // 1. Corrupted database stores invalid height + // 2. Arithmetic on heights near the limit + // 3. Could cause incorrect block selection + } + + /// This test demonstrates the overflow bug panics in debug mode. + #[test] + #[should_panic] + #[cfg(debug_assertions)] // Only runs in debug mode + fn test_overflow_panics_in_debug() { + let high_height = Height(u32::MAX - 10); + + // In debug mode, this panics with "attempt to add with overflow" + let _result = high_height + 20; + } + + /// This test demonstrates field access allows bypassing validation. + /// Height can only be constructed via TryFrom (validated), but arithmetic + /// can produce invalid values. + #[test] + #[cfg(not(debug_assertions))] + fn test_arithmetic_bypasses_validation() { + // Construction is validated + let result = Height::try_from(u32::MAX); + assert!(result.is_err()); + assert_eq!(result.unwrap_err(), "height must be ≤ 2^31 - 1"); + + // But arithmetic can create invalid heights! + let valid_height = Height(zebra_chain::block::Height::MAX.0); // 2^31 - 1 + let invalid_height = valid_height + 1; // Now we have 2^31 (invalid!) + + // This height is > MAX but exists as a Height type + assert!(invalid_height.0 > zebra_chain::block::Height::MAX.0); + + // Type system can't prevent this - the invariant is broken + } +}