From 376a41deb28c7634d0444cf2fc5a9f8e890adcec Mon Sep 17 00:00:00 2001 From: Boog900 <54e72d8a-345f-4599-bd90-c6b9bc7d0ec5@aleeas.com> Date: Sun, 17 Dec 2023 03:13:30 +0000 Subject: [PATCH] fmt + clippy + fix tests --- consensus/Cargo.toml | 2 + consensus/rules/Cargo.toml | 5 +- consensus/rules/src/hard_forks.rs | 26 ++++++--- consensus/rules/src/hard_forks/tests.rs | 69 +++++++++++++++++++++++ consensus/rules/src/lib.rs | 2 +- consensus/src/bin/scan_chain.rs | 2 +- consensus/src/block.rs | 2 +- consensus/src/context/hardforks.rs | 13 +---- consensus/src/context/hardforks/tests.rs | 71 ++---------------------- consensus/src/helper.rs | 15 ----- 10 files changed, 101 insertions(+), 106 deletions(-) create mode 100644 consensus/rules/src/hard_forks/tests.rs diff --git a/consensus/Cargo.toml b/consensus/Cargo.toml index 550f30d2..3358ffcf 100644 --- a/consensus/Cargo.toml +++ b/consensus/Cargo.toml @@ -65,6 +65,8 @@ syn = "2.0.37" [dev-dependencies] +monero-consensus = {path = "./rules", features = ["proptest"]} + tokio = {version = "1", features = ["rt-multi-thread", "macros"]} proptest = "1" proptest-derive = "0.4.0" \ No newline at end of file diff --git a/consensus/rules/Cargo.toml b/consensus/rules/Cargo.toml index 50952f78..2cc0f52b 100644 --- a/consensus/rules/Cargo.toml +++ b/consensus/rules/Cargo.toml @@ -23,7 +23,10 @@ thiserror = { workspace = true } rayon = { workspace = true, optional = true } -# Testing proptest = {workspace = true, optional = true} proptest-derive = {workspace = true, optional = true} +[dev-dependencies] +proptest = {workspace = true} +proptest-derive = {workspace = true} +tokio = {version = "1.35.0", features = ["rt-multi-thread", "macros"]} \ No newline at end of file diff --git a/consensus/rules/src/hard_forks.rs b/consensus/rules/src/hard_forks.rs index 43faa6d7..54071169 100644 --- a/consensus/rules/src/hard_forks.rs +++ b/consensus/rules/src/hard_forks.rs @@ -4,7 +4,7 @@ //! an identifier for every current hard-fork. //! //! This module also contains a [`HFVotes`] struct which keeps track of current blockchain voting, and -//! has a method [`HFVotes::check_next_hard_fork`] to check if the next hard-fork should be activated. +//! has a method [`HFVotes::current_fork`] to check if the next hard-fork should be activated. //! use monero_serai::block::BlockHeader; use std::{ @@ -13,12 +13,15 @@ use std::{ time::Duration, }; +#[cfg(test)] +mod tests; + /// Target block time for hf 1. const BLOCK_TIME_V1: Duration = Duration::from_secs(60); /// Target block time from v2. const BLOCK_TIME_V2: Duration = Duration::from_secs(120); -const NUMB_OF_HARD_FORKS: usize = 16; +pub const NUMB_OF_HARD_FORKS: usize = 16; #[derive(Debug, Copy, Clone, PartialEq, Eq, thiserror::Error)] pub enum HardForkError { @@ -51,6 +54,10 @@ impl HFsInfo { self.0[*hf as usize - 1] } + pub const fn new(hfs: [HFInfo; NUMB_OF_HARD_FORKS]) -> Self { + Self(hfs) + } + /// Returns the main-net hard-fork information. /// /// https://cuprate.github.io/monero-book/consensus_rules/hardforks.html#Mainnet-Hard-Forks @@ -78,7 +85,7 @@ impl HFsInfo { /// An identifier for every hard-fork Monero has had. #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Copy, Clone)] -#[cfg_attr(proptest, derive(proptest_derive::Arbitrary))] +#[cfg_attr(any(feature = "proptest", test), derive(proptest_derive::Arbitrary))] #[repr(u8)] pub enum HardFork { V1 = 1, @@ -245,29 +252,30 @@ impl HFVotes { /// activated. /// /// https://cuprate.github.io/monero-book/consensus_rules/hardforks.html#accepting-a-fork - pub fn check_next_hard_fork( + pub fn current_fork( &self, current_hf: &HardFork, current_height: u64, window: u64, hfs_info: &HFsInfo, - ) -> Option { - let mut approved_next_hf = None; + ) -> HardFork { + let mut current_hf = *current_hf; + while let Some(next_hf) = current_hf.next_fork() { let hf_info = hfs_info.info_for_hf(&next_hf); if current_height >= hf_info.height && self.votes_for_hf(&next_hf) >= votes_needed(hf_info.threshold, window) { - approved_next_hf = Some(next_hf); + current_hf = next_hf; } else { // if we don't have enough votes for this fork any future fork won't have enough votes // as votes are cumulative. // TODO: If a future fork has a lower threshold that could not be true, but as all current forks // have threshold 0 it is ok for now. - return approved_next_hf; + return current_hf; } } - approved_next_hf + current_hf } } diff --git a/consensus/rules/src/hard_forks/tests.rs b/consensus/rules/src/hard_forks/tests.rs new file mode 100644 index 00000000..4090c5ee --- /dev/null +++ b/consensus/rules/src/hard_forks/tests.rs @@ -0,0 +1,69 @@ +use std::convert::TryInto; + +use proptest::{arbitrary::any, prop_assert_eq, prop_compose, proptest}; + +use crate::hard_forks::{HFVotes, HardFork, NUMB_OF_HARD_FORKS}; + +const TEST_WINDOW_SIZE: u64 = 25; + +#[test] +fn next_hard_forks() { + let mut prev = HardFork::V1; + let mut next = HardFork::V2; + for _ in 2..NUMB_OF_HARD_FORKS { + assert!(prev < next); + prev = next; + next = next.next_fork().unwrap(); + } +} + +#[test] +fn hard_forks_defined() { + for fork in 1..=NUMB_OF_HARD_FORKS { + HardFork::from_version(fork.try_into().unwrap()).unwrap(); + } +} + +prop_compose! { + /// Generates an arbitrary full [`HFVotes`]. + fn arb_full_hf_votes() + ( + // we can't use HardFork as for some reason it overflows the stack, so we use u8. + votes in any::<[u8; TEST_WINDOW_SIZE as usize]>() + ) -> HFVotes { + let mut vote_count = HFVotes::new(TEST_WINDOW_SIZE as usize); + for vote in votes { + vote_count.add_vote_for_hf(&HardFork::from_vote(vote % 17)); + } + vote_count + } +} + +proptest! { + #[test] + fn hf_vote_counter_total_correct(hf_votes in arb_full_hf_votes()) { + prop_assert_eq!(hf_votes.total_votes(), u64::try_from(hf_votes.vote_list.len()).unwrap()); + + let mut votes = [0_u64; NUMB_OF_HARD_FORKS]; + for vote in hf_votes.vote_list.iter() { + // manually go through the list of votes tallying + votes[*vote as usize - 1] += 1; + } + + prop_assert_eq!(votes, hf_votes.votes); + } + + #[test] + fn window_size_kept_constant(mut hf_votes in arb_full_hf_votes(), new_votes in any::>()) { + for new_vote in new_votes.into_iter() { + hf_votes.add_vote_for_hf(&new_vote); + prop_assert_eq!(hf_votes.total_votes(), TEST_WINDOW_SIZE) + } + } + + #[test] + fn votes_out_of_range(high_vote in (NUMB_OF_HARD_FORKS+ 1).try_into().unwrap()..u8::MAX) { + prop_assert_eq!(HardFork::from_vote(0), HardFork::V1); + prop_assert_eq!(HardFork::from_vote(u8::try_from(NUMB_OF_HARD_FORKS).unwrap() + 1_u8), HardFork::from_vote(high_vote)); + } +} diff --git a/consensus/rules/src/lib.rs b/consensus/rules/src/lib.rs index 5029aa6b..04c5adbf 100644 --- a/consensus/rules/src/lib.rs +++ b/consensus/rules/src/lib.rs @@ -3,7 +3,7 @@ use std::time::{SystemTime, UNIX_EPOCH}; pub mod blocks; mod decomposed_amount; pub mod genesis; -mod hard_forks; +pub mod hard_forks; pub mod miner_tx; pub mod signatures; pub mod transactions; diff --git a/consensus/src/bin/scan_chain.rs b/consensus/src/bin/scan_chain.rs index 5df30142..174b2c21 100644 --- a/consensus/src/bin/scan_chain.rs +++ b/consensus/src/bin/scan_chain.rs @@ -171,7 +171,7 @@ where }); while let Some(incoming_blocks) = incoming_blocks.next().await { - let mut height = 0; + let mut height; for block in incoming_blocks { let VerifyBlockResponse::MainChain(verified_block_info) = block_verifier .ready() diff --git a/consensus/src/block.rs b/consensus/src/block.rs index 2765cd10..067bac88 100644 --- a/consensus/src/block.rs +++ b/consensus/src/block.rs @@ -6,7 +6,7 @@ use std::{ }; use futures::FutureExt; -use monero_serai::{block::Block, transaction::Input}; +use monero_serai::block::Block; use tower::{Service, ServiceExt}; use monero_consensus::{ diff --git a/consensus/src/context/hardforks.rs b/consensus/src/context/hardforks.rs index 8f11b942..a5a41d2c 100644 --- a/consensus/src/context/hardforks.rs +++ b/consensus/src/context/hardforks.rs @@ -119,14 +119,12 @@ impl HardForkState { /// /// https://cuprate.github.io/monero-docs/consensus_rules/hardforks.html#accepting-a-fork fn check_set_new_hf(&mut self) { - if let Some(next_fork) = self.votes.check_next_hard_fork( + self.current_hardfork = self.votes.current_fork( &self.current_hardfork, self.last_height + 1, self.config.window, &self.config.info, - ) { - self.current_hardfork = next_fork; - } + ); } pub fn current_hardfork(&self) -> HardFork { @@ -134,13 +132,6 @@ impl HardForkState { } } -/// Returns the votes needed for this fork. -/// -/// https://cuprate.github.io/monero-docs/consensus_rules/hardforks.html#accepting-a-fork -pub fn votes_needed(threshold: u64, window: u64) -> u64 { - (threshold * window + 99) / 100 -} - #[instrument(name = "get_votes", skip(database))] async fn get_votes_in_range( database: D, diff --git a/consensus/src/context/hardforks/tests.rs b/consensus/src/context/hardforks/tests.rs index 21c8688d..7390bec7 100644 --- a/consensus/src/context/hardforks/tests.rs +++ b/consensus/src/context/hardforks/tests.rs @@ -1,8 +1,7 @@ -use std::convert::TryInto; +use monero_consensus::hard_forks::{HFInfo, HardFork, NUMB_OF_HARD_FORKS}; +use monero_consensus::HFsInfo; -use proptest::{arbitrary::any, prop_assert_eq, prop_compose, proptest}; - -use super::{HFInfo, HFVotes, HardFork, HardForkConfig, HardForkState, NUMB_OF_HARD_FORKS}; +use super::{HardForkConfig, HardForkState}; use crate::test_utils::mock_db::*; const TEST_WINDOW_SIZE: u64 = 25; @@ -28,27 +27,9 @@ const TEST_HFS: [HFInfo; NUMB_OF_HARD_FORKS] = [ pub const TEST_HARD_FORK_CONFIG: HardForkConfig = HardForkConfig { window: TEST_WINDOW_SIZE, - forks: TEST_HFS, + info: HFsInfo::new(TEST_HFS), }; -#[test] -fn next_hard_forks() { - let mut prev = HardFork::V1; - let mut next = HardFork::V2; - for _ in 2..NUMB_OF_HARD_FORKS { - assert!(prev < next); - prev = next; - next = next.next_fork().unwrap(); - } -} - -#[test] -fn hard_forks_defined() { - for fork in 1..=NUMB_OF_HARD_FORKS { - HardFork::from_version(&fork.try_into().unwrap()).unwrap(); - } -} - #[tokio::test] async fn hard_fork_set_depends_on_top_block() { let mut db_builder = DummyDatabaseBuilder::default(); @@ -72,47 +53,3 @@ async fn hard_fork_set_depends_on_top_block() { assert_eq!(state.current_hardfork, HardFork::V14); } - -prop_compose! { - /// Generates an arbitrary full [`HFVotes`]. - fn arb_full_hf_votes() - ( - // we can't use HardFork as for some reason it overflows the stack, so we use u8. - votes in any::<[u8; TEST_WINDOW_SIZE as usize]>() - ) -> HFVotes { - let mut vote_count = HFVotes::new(TEST_WINDOW_SIZE as usize); - for vote in votes { - vote_count.add_vote_for_hf(&HardFork::from_vote(&(vote % 17))); - } - vote_count - } -} - -proptest! { - #[test] - fn hf_vote_counter_total_correct(hf_votes in arb_full_hf_votes()) { - prop_assert_eq!(hf_votes.total_votes(), u64::try_from(hf_votes.vote_list.len()).unwrap()); - - let mut votes = [0_u64; NUMB_OF_HARD_FORKS]; - for vote in hf_votes.vote_list.iter() { - // manually go through the list of votes tallying - votes[*vote as usize - 1] += 1; - } - - prop_assert_eq!(votes, hf_votes.votes); - } - - #[test] - fn window_size_kept_constant(mut hf_votes in arb_full_hf_votes(), new_votes in any::>()) { - for new_vote in new_votes.into_iter() { - hf_votes.add_vote_for_hf(&new_vote); - prop_assert_eq!(hf_votes.total_votes(), TEST_WINDOW_SIZE) - } - } - - #[test] - fn votes_out_of_range(high_vote in (NUMB_OF_HARD_FORKS+ 1).try_into().unwrap()..u8::MAX) { - prop_assert_eq!(HardFork::from_vote(&0), HardFork::V1); - prop_assert_eq!(HardFork::from_vote(&NUMB_OF_HARD_FORKS.try_into().unwrap()), HardFork::from_vote(&high_vote)); - } -} diff --git a/consensus/src/helper.rs b/consensus/src/helper.rs index ca7ba0af..3cf93aa5 100644 --- a/consensus/src/helper.rs +++ b/consensus/src/helper.rs @@ -3,8 +3,6 @@ use std::{ time::{SystemTime, UNIX_EPOCH}, }; -use curve25519_dalek::edwards::CompressedEdwardsY; - /// Spawns a task for the rayon thread pool and awaits the result without blocking the async runtime. pub(crate) async fn rayon_spawn_async(f: F) -> R where @@ -54,16 +52,3 @@ pub(crate) fn current_time() -> u64 { .unwrap() .as_secs() } - -/// Checks that a point is canonical. -/// -/// https://github.com/dalek-cryptography/curve25519-dalek/issues/380 -pub(crate) fn check_point(point: &CompressedEdwardsY) -> bool { - let bytes = point.as_bytes(); - - point - .decompress() - // Ban points which are either unreduced or -0 - .filter(|point| point.compress().as_bytes() == bytes) - .is_some() -}