From 082f58cdeee93561bf15fe2164057b361147f459 Mon Sep 17 00:00:00 2001 From: brentstone Date: Tue, 3 Sep 2024 22:18:10 -0500 Subject: [PATCH] fix validator voting period computation --- crates/governance/src/utils.rs | 55 ++++++++++++++++++++++++---------- 1 file changed, 39 insertions(+), 16 deletions(-) diff --git a/crates/governance/src/utils.rs b/crates/governance/src/utils.rs index 7f05a220746..5221173e644 100644 --- a/crates/governance/src/utils.rs +++ b/crates/governance/src/utils.rs @@ -8,6 +8,7 @@ use namada_core::chain::Epoch; use namada_core::collections::HashMap; use namada_core::dec::Dec; use namada_core::token; +use namada_core::uint::Uint; use namada_macros::BorshDeserializer; #[cfg(feature = "migrations")] use namada_migrations::*; @@ -422,10 +423,10 @@ pub fn compute_proposal_result( } /// Calculate the valid voting window for a validator given proposal epoch -/// details. The valid window is within 2/3 of the voting period. -/// NOTE: technically the window can be more generous than 2/3 since the end -/// epoch is a valid epoch for voting too. -/// Returns `false` if any arithmetic fails. +/// details. The valid window is approximately within 2/3 of the voting period. +/// NOTE: technically the window can be more generous than 2/3, especially for +/// low `voting_end_epoch - voting_start_epoch`, but it will never be less than +/// 2/3. Returns `false` if any arithmetic fails. pub fn is_valid_validator_voting_period( current_epoch: Epoch, voting_start_epoch: Epoch, @@ -433,11 +434,18 @@ pub fn is_valid_validator_voting_period( ) -> bool { if voting_start_epoch >= voting_end_epoch { false + } else if voting_end_epoch + .0 + .checked_sub(voting_start_epoch.0) + .unwrap() // safe unwrap + <= 2 + { + current_epoch == voting_start_epoch } else { (|| { - // From e_cur <= e_start + 2/3 * (e_end - e_start) + // From e_cur < e_start + 2/3 * (e_end - e_start) let is_within_two_thirds = checked!( - current_epoch * 3 <= voting_start_epoch + voting_end_epoch * 2 + current_epoch * 3 < voting_start_epoch + voting_end_epoch * 2 ) .ok()?; @@ -455,13 +463,26 @@ pub fn last_validator_voting_epoch( voting_end_epoch: Epoch, ) -> Result, arith::Error> { if voting_start_epoch >= voting_end_epoch { - Ok(None) + return Ok(None); + } + + #[allow(clippy::arithmetic_side_effects)] + let diff_epochs = voting_end_epoch.0 - voting_start_epoch.0; + + if diff_epochs <= 2 { + Ok(Some(voting_start_epoch)) } else { - let latest = checked!( - voting_start_epoch.0 - + 2u64 * (voting_end_epoch.0 - voting_start_epoch.0) / 3u64 - )?; - Ok(Some(Epoch(latest))) + let diff_epochs = Uint::from_u64(diff_epochs); + // ceiling multiplication by 2/3 + let valid_period = + u64::try_from(diff_epochs.frac_mul_ceil(2.into(), 3.into())?).ok(); + + if let Some(period) = valid_period { + let latest = checked!(voting_start_epoch + period - 1u64)?; + Ok(Some(latest)) + } else { + Ok(None) + } } } @@ -1438,6 +1459,7 @@ mod test { #[test] fn test_validator_voting_period() { + // Voting period of 2 epochs assert!(!is_valid_validator_voting_period( 0.into(), 2.into(), @@ -1448,7 +1470,7 @@ mod test { 2.into(), 4.into() )); - assert!(is_valid_validator_voting_period( + assert!(!is_valid_validator_voting_period( 3.into(), 2.into(), 4.into() @@ -1462,15 +1484,16 @@ mod test { last_validator_voting_epoch(2.into(), 4.into()) .unwrap() .unwrap(), - 3.into() + 2.into() ); + // Voting period of 3 epochs assert!(is_valid_validator_voting_period( 3.into(), 2.into(), 5.into() )); - assert!(is_valid_validator_voting_period( + assert!(!is_valid_validator_voting_period( 4.into(), 2.into(), 5.into() @@ -1484,7 +1507,7 @@ mod test { last_validator_voting_epoch(2.into(), 5.into()) .unwrap() .unwrap(), - 4.into() + 3.into() ); for end_epoch in 1u64..=20 {