From 65ac844cb2352ca54bf927af3a71507b9c5952f3 Mon Sep 17 00:00:00 2001 From: Roman Krasiuk Date: Tue, 15 Nov 2022 13:47:15 +0200 Subject: [PATCH 01/17] test(sync): stage test suite --- crates/interfaces/src/test_utils/headers.rs | 9 + crates/stages/src/stages/headers.rs | 388 ++++++++++---------- crates/stages/src/stages/tx_index.rs | 122 +++--- crates/stages/src/util.rs | 166 +++++++-- 4 files changed, 423 insertions(+), 262 deletions(-) diff --git a/crates/interfaces/src/test_utils/headers.rs b/crates/interfaces/src/test_utils/headers.rs index ddd9c0d1388..50bdcf270cf 100644 --- a/crates/interfaces/src/test_utils/headers.rs +++ b/crates/interfaces/src/test_utils/headers.rs @@ -92,6 +92,12 @@ impl TestHeadersClient { pub fn send_header_response(&self, id: u64, headers: Vec
) { self.res_tx.send((id, headers).into()).expect("failed to send header response"); } + + /// Helper for pushing responses to the client + pub async fn send_header_response_delayed(&self, id: u64, headers: Vec
, secs: u64) { + tokio::time::sleep(Duration::from_secs(secs)).await; + self.send_header_response(id, headers); + } } #[async_trait::async_trait] @@ -105,6 +111,9 @@ impl HeadersClient for TestHeadersClient { } async fn stream_headers(&self) -> HeadersStream { + if !self.res_rx.is_empty() { + println!("WARNING: broadcast receiver already contains messages.") + } Box::pin(BroadcastStream::new(self.res_rx.resubscribe()).filter_map(|e| e.ok())) } } diff --git a/crates/stages/src/stages/headers.rs b/crates/stages/src/stages/headers.rs index f24c81547cd..e9e0d09ef05 100644 --- a/crates/stages/src/stages/headers.rs +++ b/crates/stages/src/stages/headers.rs @@ -182,223 +182,160 @@ impl HeaderStage { #[cfg(test)] mod tests { use super::*; - use crate::util::test_utils::StageTestRunner; + use crate::util::test_utils::{ + stage_test_suite, ExecuteStageTestRunner, UnwindStageTestRunner, PREV_STAGE_ID, + }; use assert_matches::assert_matches; use reth_interfaces::test_utils::{gen_random_header, gen_random_header_range}; - use test_utils::{HeadersTestRunner, TestDownloader}; + use test_runner::HeadersTestRunner; - const TEST_STAGE: StageId = StageId("Headers"); + stage_test_suite!(HeadersTestRunner); #[tokio::test] - // Check that the execution errors on empty database or - // prev progress missing from the database. - async fn execute_empty_db() { - let runner = HeadersTestRunner::default(); - let rx = runner.execute(ExecInput::default()); - assert_matches!( - rx.await.unwrap(), - Err(StageError::DatabaseIntegrity(DatabaseIntegrityError::CannonicalHeader { .. })) - ); - } - - #[tokio::test] - // Check that the execution exits on downloader timeout. + // Validate that the execution does not fail on timeout async fn execute_timeout() { - let head = gen_random_header(0, None); - let runner = - HeadersTestRunner::with_downloader(TestDownloader::new(Err(DownloadError::Timeout { - request_id: 0, - }))); - runner.insert_header(&head).expect("failed to insert header"); - - let rx = runner.execute(ExecInput::default()); - runner.consensus.update_tip(H256::from_low_u64_be(1)); - assert_matches!(rx.await.unwrap(), Ok(ExecOutput { done, .. }) if !done); - } - - #[tokio::test] - // Check that validation error is propagated during the execution. - async fn execute_validation_error() { - let head = gen_random_header(0, None); - let runner = HeadersTestRunner::with_downloader(TestDownloader::new(Err( - DownloadError::HeaderValidation { hash: H256::zero(), details: "".to_owned() }, - ))); - runner.insert_header(&head).expect("failed to insert header"); - - let rx = runner.execute(ExecInput::default()); - runner.consensus.update_tip(H256::from_low_u64_be(1)); - assert_matches!(rx.await.unwrap(), Err(StageError::Validation { block }) if block == 0); - } - - #[tokio::test] - // Validate that all necessary tables are updated after the - // header download on no previous progress. - async fn execute_no_progress() { - let (start, end) = (0, 100); - let head = gen_random_header(start, None); - let headers = gen_random_header_range(start + 1..end, head.hash()); - - let result = headers.iter().rev().cloned().collect::>(); - let runner = HeadersTestRunner::with_downloader(TestDownloader::new(Ok(result))); - runner.insert_header(&head).expect("failed to insert header"); + let mut runner = HeadersTestRunner::default(); + let (stage_progress, previous_stage) = (0, 0); + runner + .seed_execution(ExecInput { + previous_stage: Some((PREV_STAGE_ID, previous_stage)), + stage_progress: Some(stage_progress), + }) + .expect("failed to seed execution"); let rx = runner.execute(ExecInput::default()); - let tip = headers.last().unwrap(); - runner.consensus.update_tip(tip.hash()); - + runner.after_execution().await.expect("failed to run after execution hook"); assert_matches!( rx.await.unwrap(), - Ok(ExecOutput { done, reached_tip, stage_progress }) - if done && reached_tip && stage_progress == tip.number + Ok(ExecOutput { done, reached_tip, stage_progress: out_stage_progress }) + if !done && !reached_tip && out_stage_progress == stage_progress ); - assert!(headers.iter().try_for_each(|h| runner.validate_db_header(&h)).is_ok()); + assert!(runner.validate_execution().is_ok(), "validation failed"); } #[tokio::test] // Validate that all necessary tables are updated after the // header download with some previous progress. async fn execute_prev_progress() { - let (start, end) = (10000, 10241); - let head = gen_random_header(start, None); - let headers = gen_random_header_range(start + 1..end, head.hash()); - - let result = headers.iter().rev().cloned().collect::>(); - let runner = HeadersTestRunner::with_downloader(TestDownloader::new(Ok(result))); - runner.insert_header(&head).expect("failed to insert header"); - - let rx = runner.execute(ExecInput { - previous_stage: Some((TEST_STAGE, head.number)), - stage_progress: Some(head.number), - }); - let tip = headers.last().unwrap(); - runner.consensus.update_tip(tip.hash()); - - assert_matches!( - rx.await.unwrap(), - Ok(ExecOutput { done, reached_tip, stage_progress }) - if done && reached_tip && stage_progress == tip.number - ); - assert!(headers.iter().try_for_each(|h| runner.validate_db_header(&h)).is_ok()); - } - - #[tokio::test] - // Execute the stage with linear downloader - async fn execute_with_linear_downloader() { - let (start, end) = (1000, 1200); - let head = gen_random_header(start, None); - let headers = gen_random_header_range(start + 1..end, head.hash()); - - let runner = HeadersTestRunner::with_linear_downloader(); - runner.insert_header(&head).expect("failed to insert header"); - let rx = runner.execute(ExecInput { - previous_stage: Some((TEST_STAGE, head.number)), - stage_progress: Some(head.number), - }); - - let tip = headers.last().unwrap(); - runner.consensus.update_tip(tip.hash()); - - let mut download_result = headers.clone(); - download_result.insert(0, head); - runner - .client - .on_header_request(1, |id, _| { - runner.client.send_header_response( - id, - download_result.clone().into_iter().map(|h| h.unseal()).collect(), - ) - }) - .await; - + let mut runner = HeadersTestRunner::default(); + let (stage_progress, previous_stage) = (10000, 10241); + let input = ExecInput { + previous_stage: Some((PREV_STAGE_ID, previous_stage)), + stage_progress: Some(stage_progress), + }; + runner.seed_execution(input).expect("failed to seed execution"); + let rx = runner.execute(input); + runner.after_execution().await.expect("failed to run after execution hook"); assert_matches!( rx.await.unwrap(), Ok(ExecOutput { done, reached_tip, stage_progress }) - if done && reached_tip && stage_progress == tip.number + if done && reached_tip && stage_progress == stage_progress ); - assert!(headers.iter().try_for_each(|h| runner.validate_db_header(&h)).is_ok()); + assert!(runner.validate_execution().is_ok(), "validation failed"); } - #[tokio::test] - // Check that unwind does not panic on empty database. - async fn unwind_empty_db() { - let unwind_to = 100; - let runner = HeadersTestRunner::default(); - let rx = - runner.unwind(UnwindInput { bad_block: None, stage_progress: unwind_to, unwind_to }); - assert_matches!( - rx.await.unwrap(), - Ok(UnwindOutput {stage_progress} ) if stage_progress == unwind_to - ); - } - - #[tokio::test] - // Check that unwind can remove headers across gaps - async fn unwind_db_gaps() { - let runner = HeadersTestRunner::default(); - let head = gen_random_header(0, None); - let first_range = gen_random_header_range(1..20, head.hash()); - let second_range = gen_random_header_range(50..100, H256::zero()); - runner.insert_header(&head).expect("failed to insert header"); - runner - .insert_headers(first_range.iter().chain(second_range.iter())) - .expect("failed to insert headers"); - - let unwind_to = 15; - let rx = - runner.unwind(UnwindInput { bad_block: None, stage_progress: unwind_to, unwind_to }); - assert_matches!( - rx.await.unwrap(), - Ok(UnwindOutput {stage_progress} ) if stage_progress == unwind_to - ); - - runner - .db() - .check_no_entry_above::(unwind_to, |key| key) - .expect("failed to check cannonical headers"); - runner - .db() - .check_no_entry_above_by_value::(unwind_to, |val| val) - .expect("failed to check header numbers"); - runner - .db() - .check_no_entry_above::(unwind_to, |key| key.number()) - .expect("failed to check headers"); - runner - .db() - .check_no_entry_above::(unwind_to, |key| key.number()) - .expect("failed to check td"); - } - - mod test_utils { + // TODO: + // #[tokio::test] + // // Execute the stage with linear downloader + // async fn execute_with_linear_downloader() { + // let mut runner = HeadersTestRunner::with_linear_downloader(); + // let (stage_progress, previous_stage) = (1000, 1200); + // let input = ExecInput { + // previous_stage: Some((PREV_STAGE_ID, previous_stage)), + // stage_progress: Some(stage_progress), + // }; + // runner.seed_execution(input).expect("failed to seed execution"); + // let rx = runner.execute(input); + + // // skip hook for linear downloader + // let headers = runner.context.as_ref().unwrap(); + // let tip = headers.first().unwrap(); + // runner.consensus.update_tip(tip.hash()); + + // // TODO: + // let mut download_result = headers.clone(); + // download_result.insert(0, headers.last().unwrap().clone()); + // runner + // .client + // .on_header_request(1, |id, _| { + // let response = download_result.clone().into_iter().map(|h| h.unseal()).collect(); + // runner.client.send_header_response(id, response) + // }) + // .await; + + // assert_matches!( + // rx.await.unwrap(), + // Ok(ExecOutput { done, reached_tip, stage_progress }) + // if done && reached_tip && stage_progress == tip.number + // ); + // assert!(runner.validate_execution().is_ok(), "validation failed"); + // } + + // TODO: + // #[tokio::test] + // // Check that unwind can remove headers across gaps + // async fn unwind_db_gaps() { + // let runner = HeadersTestRunner::default(); + // let head = gen_random_header(0, None); + // let first_range = gen_random_header_range(1..20, head.hash()); + // let second_range = gen_random_header_range(50..100, H256::zero()); + // runner.insert_header(&head).expect("failed to insert header"); + // runner + // .insert_headers(first_range.iter().chain(second_range.iter())) + // .expect("failed to insert headers"); + + // let unwind_to = 15; + // let input = UnwindInput { bad_block: None, stage_progress: unwind_to, unwind_to }; + // let rx = runner.unwind(input); + // assert_matches!( + // rx.await.unwrap(), + // Ok(UnwindOutput {stage_progress} ) if stage_progress == unwind_to + // ); + // assert!(runner.validate_unwind(input).is_ok(), "validation failed"); + // } + + mod test_runner { use crate::{ stages::headers::HeaderStage, - util::test_utils::{StageTestDB, StageTestRunner}, + util::test_utils::{ + ExecuteStageTestRunner, StageTestDB, StageTestRunner, UnwindStageTestRunner, + }, + ExecInput, UnwindInput, }; use async_trait::async_trait; use reth_headers_downloaders::linear::{LinearDownloadBuilder, LinearDownloader}; use reth_interfaces::{ consensus::ForkchoiceState, db::{self, models::blocks::BlockNumHash, tables, DbTx}, - p2p::headers::downloader::{DownloadError, Downloader}, - test_utils::{TestConsensus, TestHeadersClient}, + p2p::headers::{ + client::HeadersClient, + downloader::{DownloadError, Downloader}, + }, + test_utils::{ + gen_random_header, gen_random_header_range, TestConsensus, TestHeadersClient, + }, }; use reth_primitives::{rpc::BigEndianHash, SealedHeader, H256, U256}; use std::{ops::Deref, sync::Arc, time::Duration}; + use tokio_stream::StreamExt; pub(crate) struct HeadersTestRunner { pub(crate) consensus: Arc, pub(crate) client: Arc, + pub(crate) context: Option>, downloader: Arc, db: StageTestDB, } impl Default for HeadersTestRunner { fn default() -> Self { + let client = Arc::new(TestHeadersClient::default()); Self { - client: Arc::new(TestHeadersClient::default()), + client: client.clone(), consensus: Arc::new(TestConsensus::default()), - downloader: Arc::new(TestDownloader::new(Ok(Vec::default()))), + downloader: Arc::new(TestDownloader::new(client)), db: StageTestDB::default(), + context: None, } } } @@ -419,26 +356,99 @@ mod tests { } } + #[async_trait::async_trait] + impl ExecuteStageTestRunner for HeadersTestRunner { + fn seed_execution( + &mut self, + input: ExecInput, + ) -> Result<(), Box> { + let start = input.stage_progress.unwrap_or_default(); + let head = gen_random_header(start, None); + self.insert_header(&head)?; + + // use previous progress as seed size + let end = input.previous_stage.map(|(_, num)| num).unwrap_or_default() + 1; + if end > start + 1 { + let mut headers = gen_random_header_range(start + 1..end, head.hash()); + headers.reverse(); + self.context = Some(headers); + } + Ok(()) + } + + async fn after_execution( + &self, + ) -> Result<(), Box> { + let (tip, headers) = match self.context { + Some(ref headers) if headers.len() > 1 => { + // headers are in reverse + (headers.first().unwrap().hash(), headers.clone()) + } + _ => (H256::from_low_u64_be(rand::random()), Vec::default()), + }; + self.consensus.update_tip(tip); + if !headers.is_empty() { + self.client + .send_header_response_delayed( + 0, + headers.iter().cloned().map(|h| h.unseal()).collect(), + 1, + ) + .await; + } + Ok(()) + } + + fn validate_execution(&self) -> Result<(), Box> { + if let Some(ref headers) = self.context { + headers.iter().try_for_each(|h| self.validate_db_header(&h))?; + } + Ok(()) + } + } + + impl UnwindStageTestRunner for HeadersTestRunner { + fn seed_unwind( + &mut self, + input: UnwindInput, + highest_entry: u64, + ) -> Result<(), Box> { + let lowest_entry = input.unwind_to.saturating_sub(100); + let headers = gen_random_header_range(lowest_entry..highest_entry, H256::zero()); + self.insert_headers(headers.iter())?; + Ok(()) + } + + fn validate_unwind( + &self, + input: UnwindInput, + ) -> Result<(), Box> { + let unwind_to = input.unwind_to; + self.db().check_no_entry_above_by_value::( + unwind_to, + |val| val, + )?; + self.db() + .check_no_entry_above::(unwind_to, |key| key)?; + self.db() + .check_no_entry_above::(unwind_to, |key| key.number())?; + self.db() + .check_no_entry_above::(unwind_to, |key| key.number())?; + Ok(()) + } + } + impl HeadersTestRunner> { pub(crate) fn with_linear_downloader() -> Self { let client = Arc::new(TestHeadersClient::default()); let consensus = Arc::new(TestConsensus::default()); let downloader = Arc::new(LinearDownloadBuilder::new().build(consensus.clone(), client.clone())); - Self { client, consensus, downloader, db: StageTestDB::default() } + Self { client, consensus, downloader, db: StageTestDB::default(), context: None } } } impl HeadersTestRunner { - pub(crate) fn with_downloader(downloader: D) -> Self { - HeadersTestRunner { - client: Arc::new(TestHeadersClient::default()), - consensus: Arc::new(TestConsensus::default()), - downloader: Arc::new(downloader), - db: StageTestDB::default(), - } - } - /// Insert header into tables pub(crate) fn insert_header(&self, header: &SealedHeader) -> Result<(), db::Error> { self.insert_headers(std::iter::once(header)) @@ -504,12 +514,12 @@ mod tests { #[derive(Debug)] pub(crate) struct TestDownloader { - result: Result, DownloadError>, + client: Arc, } impl TestDownloader { - pub(crate) fn new(result: Result, DownloadError>) -> Self { - Self { result } + pub(crate) fn new(client: Arc) -> Self { + Self { client } } } @@ -527,7 +537,7 @@ mod tests { } fn client(&self) -> &Self::Client { - unimplemented!() + &self.client } async fn download( @@ -535,7 +545,15 @@ mod tests { _: &SealedHeader, _: &ForkchoiceState, ) -> Result, DownloadError> { - self.result.clone() + let stream = self.client.stream_headers().await; + let stream = stream.timeout(Duration::from_secs(3)); + + match Box::pin(stream).try_next().await { + Ok(Some(res)) => { + Ok(res.headers.iter().map(|h| h.clone().seal()).collect::>()) + } + _ => Err(DownloadError::Timeout { request_id: 0 }), + } } } } diff --git a/crates/stages/src/stages/tx_index.rs b/crates/stages/src/stages/tx_index.rs index a2d986ba714..05b45cb383f 100644 --- a/crates/stages/src/stages/tx_index.rs +++ b/crates/stages/src/stages/tx_index.rs @@ -87,22 +87,15 @@ impl Stage for TxIndex { #[cfg(test)] mod tests { use super::*; - use crate::util::test_utils::{StageTestDB, StageTestRunner}; + use crate::util::test_utils::{ + stage_test_suite, ExecuteStageTestRunner, StageTestDB, StageTestRunner, + UnwindStageTestRunner, PREV_STAGE_ID, + }; use assert_matches::assert_matches; use reth_interfaces::{db::models::BlockNumHash, test_utils::gen_random_header_range}; use reth_primitives::H256; - const TEST_STAGE: StageId = StageId("PrevStage"); - - #[tokio::test] - async fn execute_empty_db() { - let runner = TxIndexTestRunner::default(); - let rx = runner.execute(ExecInput::default()); - assert_matches!( - rx.await.unwrap(), - Err(StageError::DatabaseIntegrity(DatabaseIntegrityError::CannonicalHeader { .. })) - ); - } + stage_test_suite!(TxIndexTestRunner); #[tokio::test] async fn execute_no_prev_tx_count() { @@ -115,7 +108,7 @@ mod tests { let (head, tail) = (headers.first().unwrap(), headers.last().unwrap()); let input = ExecInput { - previous_stage: Some((TEST_STAGE, tail.number)), + previous_stage: Some((PREV_STAGE_ID, tail.number)), stage_progress: Some(head.number), }; let rx = runner.execute(input); @@ -125,48 +118,6 @@ mod tests { ); } - #[tokio::test] - async fn execute() { - let runner = TxIndexTestRunner::default(); - let (start, pivot, end) = (0, 100, 200); - let headers = gen_random_header_range(start..end, H256::zero()); - runner - .db() - .map_put::(&headers, |h| (h.number, h.hash())) - .expect("failed to insert"); - runner - .db() - .transform_append::(&headers[..=pivot], |prev, h| { - ( - BlockNumHash((h.number, h.hash())), - prev.unwrap_or_default() + (rand::random::() as u64), - ) - }) - .expect("failed to insert"); - - let (pivot, tail) = (headers.get(pivot).unwrap(), headers.last().unwrap()); - let input = ExecInput { - previous_stage: Some((TEST_STAGE, tail.number)), - stage_progress: Some(pivot.number), - }; - let rx = runner.execute(input); - assert_matches!( - rx.await.unwrap(), - Ok(ExecOutput { stage_progress, done, reached_tip }) - if done && reached_tip && stage_progress == tail.number - ); - } - - #[tokio::test] - async fn unwind_empty_db() { - let runner = TxIndexTestRunner::default(); - let rx = runner.unwind(UnwindInput::default()); - assert_matches!( - rx.await.unwrap(), - Ok(UnwindOutput { stage_progress }) if stage_progress == 0 - ); - } - #[tokio::test] async fn unwind_no_input() { let runner = TxIndexTestRunner::default(); @@ -239,4 +190,65 @@ mod tests { TxIndex {} } } + + impl ExecuteStageTestRunner for TxIndexTestRunner { + fn seed_execution( + &mut self, + input: ExecInput, + ) -> Result<(), Box> { + let pivot = input.stage_progress.unwrap_or_default(); + let start = pivot.saturating_sub(100); + let end = input.previous_stage.as_ref().map(|(_, num)| *num).unwrap_or_default(); + let headers = gen_random_header_range(start..end, H256::zero()); + self.db() + .map_put::(&headers, |h| (h.number, h.hash()))?; + self.db().transform_append::( + &headers[..=(pivot as usize)], + |prev, h| { + ( + BlockNumHash((h.number, h.hash())), + prev.unwrap_or_default() + (rand::random::() as u64), + ) + }, + )?; + Ok(()) + } + + fn validate_execution(&self) -> Result<(), Box> { + // TODO: + Ok(()) + } + } + + impl UnwindStageTestRunner for TxIndexTestRunner { + fn seed_unwind( + &mut self, + input: UnwindInput, + highest_entry: u64, + ) -> Result<(), Box> { + // TODO: accept range + let headers = gen_random_header_range(input.unwind_to..highest_entry, H256::zero()); + self.db().transform_append::( + &headers, + |prev, h| { + ( + BlockNumHash((h.number, h.hash())), + prev.unwrap_or_default() + (rand::random::() as u64), + ) + }, + )?; + Ok(()) + } + + fn validate_unwind( + &self, + input: UnwindInput, + ) -> Result<(), Box> { + self.db() + .check_no_entry_above::(input.unwind_to, |h| { + h.number() + })?; + Ok(()) + } + } } diff --git a/crates/stages/src/util.rs b/crates/stages/src/util.rs index af221916d67..a9817964fe9 100644 --- a/crates/stages/src/util.rs +++ b/crates/stages/src/util.rs @@ -139,15 +139,18 @@ pub(crate) mod unwind { #[cfg(test)] pub(crate) mod test_utils { use reth_db::{ - kv::{test_utils::create_test_db, Env, EnvKind}, - mdbx::WriteMap, + kv::{test_utils::create_test_db, tx::Tx, Env, EnvKind}, + mdbx::{WriteMap, RW}, }; use reth_interfaces::db::{DBContainer, DbCursorRO, DbCursorRW, DbTx, DbTxMut, Error, Table}; use reth_primitives::BlockNumber; use std::{borrow::Borrow, sync::Arc}; use tokio::sync::oneshot; - use crate::{ExecInput, ExecOutput, Stage, StageError, UnwindInput, UnwindOutput}; + use crate::{ExecInput, ExecOutput, Stage, StageError, StageId, UnwindInput, UnwindOutput}; + + /// The previous test stage id mock used for testing + pub(crate) const PREV_STAGE_ID: StageId = StageId("PrevStage"); /// The [StageTestDB] is used as an internal /// database for testing stage implementation. @@ -178,6 +181,30 @@ pub(crate) mod test_utils { DBContainer::new(self.db.borrow()).expect("failed to create db container") } + fn commit(&self, f: F) -> Result<(), Error> + where + F: FnOnce(&mut Tx<'_, RW, WriteMap>) -> Result<(), Error>, + { + let mut db = self.container(); + let tx = db.get_mut(); + f(tx)?; + db.commit()?; + Ok(()) + } + + /// Put a single value into the table + pub(crate) fn put(&self, k: T::Key, v: T::Value) -> Result<(), Error> { + self.commit(|tx| tx.put::(k, v)) + } + + /// Delete a single value from the table + pub(crate) fn delete(&self, k: T::Key) -> Result<(), Error> { + self.commit(|tx| { + tx.delete::(k, None)?; + Ok(()) + }) + } + /// Map a collection of values and store them in the database. /// This function commits the transaction before exiting. /// @@ -191,14 +218,12 @@ pub(crate) mod test_utils { S: Clone, F: FnMut(&S) -> (T::Key, T::Value), { - let mut db = self.container(); - let tx = db.get_mut(); - values.iter().try_for_each(|src| { - let (k, v) = map(src); - tx.put::(k, v) - })?; - db.commit()?; - Ok(()) + self.commit(|tx| { + values.iter().try_for_each(|src| { + let (k, v) = map(src); + tx.put::(k, v) + }) + }) } /// Transform a collection of values using a callback and store @@ -221,17 +246,15 @@ pub(crate) mod test_utils { S: Clone, F: FnMut(&Option<::Value>, &S) -> (T::Key, T::Value), { - let mut db = self.container(); - let tx = db.get_mut(); - let mut cursor = tx.cursor_mut::()?; - let mut last = cursor.last()?.map(|(_, v)| v); - values.iter().try_for_each(|src| { - let (k, v) = transform(&last, src); - last = Some(v.clone()); - cursor.append(k, v) - })?; - db.commit()?; - Ok(()) + self.commit(|tx| { + let mut cursor = tx.cursor_mut::()?; + let mut last = cursor.last()?.map(|(_, v)| v); + values.iter().try_for_each(|src| { + let (k, v) = transform(&last, src); + last = Some(v.clone()); + cursor.append(k, v) + }) + }) } /// Check that there is no table entry above a given @@ -289,6 +312,18 @@ pub(crate) mod test_utils { /// Return an instance of a Stage. fn stage(&self) -> Self::S; + } + + #[async_trait::async_trait] + pub(crate) trait ExecuteStageTestRunner: StageTestRunner { + /// Seed database for stage execution + fn seed_execution( + &mut self, + input: ExecInput, + ) -> Result<(), Box>; + + /// Validate stage execution + fn validate_execution(&self) -> Result<(), Box>; /// Run [Stage::execute] and return a receiver for the result. fn execute(&self, input: ExecInput) -> oneshot::Receiver> { @@ -303,6 +338,26 @@ pub(crate) mod test_utils { rx } + /// Run a hook after [Stage::execute]. Required for Headers & Bodies stages. + async fn after_execution(&self) -> Result<(), Box> { + Ok(()) + } + } + + pub(crate) trait UnwindStageTestRunner: StageTestRunner { + /// Seed database for stage unwind + fn seed_unwind( + &mut self, + input: UnwindInput, + highest_entry: u64, + ) -> Result<(), Box>; + + /// Validate the unwind + fn validate_unwind( + &self, + input: UnwindInput, + ) -> Result<(), Box>; + /// Run [Stage::unwind] and return a receiver for the result. fn unwind( &self, @@ -320,4 +375,71 @@ pub(crate) mod test_utils { rx } } + + macro_rules! stage_test_suite { + ($runner:ident) => { + #[tokio::test] + // Check that the execution errors on empty database or + // prev progress missing from the database. + async fn execute_empty_db() { + let runner = $runner::default(); + let rx = runner.execute(crate::stage::ExecInput::default()); + assert_matches!( + rx.await.unwrap(), + Err(crate::error::StageError::DatabaseIntegrity(_)) + ); + assert!(runner.validate_execution().is_ok(), "execution validation"); + } + + #[tokio::test] + async fn execute() { + let (previous_stage, stage_progress) = (1000, 100); + let mut runner = $runner::default(); + let input = crate::stage::ExecInput { + previous_stage: Some((crate::util::test_utils::PREV_STAGE_ID, previous_stage)), + stage_progress: Some(stage_progress), + }; + runner.seed_execution(input).expect("failed to seed"); + let rx = runner.execute(input); + runner.after_execution().await.expect("failed to run after execution hook"); + assert_matches!( + rx.await.unwrap(), + Ok(ExecOutput { done, reached_tip, stage_progress }) + if done && reached_tip && stage_progress == previous_stage + ); + assert!(runner.validate_execution().is_ok(), "execution validation"); + } + + #[tokio::test] + // Check that unwind does not panic on empty database. + async fn unwind_empty_db() { + let runner = $runner::default(); + let input = crate::stage::UnwindInput::default(); + let rx = runner.unwind(input); + assert_matches!( + rx.await.unwrap(), + Ok(UnwindOutput { stage_progress }) if stage_progress == input.unwind_to + ); + assert!(runner.validate_unwind(input).is_ok(), "unwind validation"); + } + + #[tokio::test] + async fn unwind() { + let (unwind_to, highest_entry) = (100, 200); + let mut runner = $runner::default(); + let input = crate::stage::UnwindInput { + unwind_to, stage_progress: unwind_to, bad_block: None, + }; + runner.seed_unwind(input, highest_entry).expect("failed to seed"); + let rx = runner.unwind(input); + assert_matches!( + rx.await.unwrap(), + Ok(UnwindOutput { stage_progress }) if stage_progress == input.unwind_to + ); + assert!(runner.validate_unwind(input).is_ok(), "unwind validation"); + } + }; + } + + pub(crate) use stage_test_suite; } From 8ee6d80949d8b8e2fea08bfbe7362f57bbd8f8c6 Mon Sep 17 00:00:00 2001 From: Roman Krasiuk Date: Tue, 15 Nov 2022 19:10:11 +0200 Subject: [PATCH 02/17] cleanup txindex tests --- crates/stages/src/stages/headers.rs | 151 ++++++++++----------------- crates/stages/src/stages/tx_index.rs | 138 +++++++++--------------- crates/stages/src/util.rs | 45 +++++--- 3 files changed, 129 insertions(+), 205 deletions(-) diff --git a/crates/stages/src/stages/headers.rs b/crates/stages/src/stages/headers.rs index e9e0d09ef05..f8a93e118eb 100644 --- a/crates/stages/src/stages/headers.rs +++ b/crates/stages/src/stages/headers.rs @@ -186,7 +186,6 @@ mod tests { stage_test_suite, ExecuteStageTestRunner, UnwindStageTestRunner, PREV_STAGE_ID, }; use assert_matches::assert_matches; - use reth_interfaces::test_utils::{gen_random_header, gen_random_header_range}; use test_runner::HeadersTestRunner; stage_test_suite!(HeadersTestRunner); @@ -195,105 +194,53 @@ mod tests { // Validate that the execution does not fail on timeout async fn execute_timeout() { let mut runner = HeadersTestRunner::default(); - let (stage_progress, previous_stage) = (0, 0); - runner - .seed_execution(ExecInput { - previous_stage: Some((PREV_STAGE_ID, previous_stage)), - stage_progress: Some(stage_progress), - }) - .expect("failed to seed execution"); - - let rx = runner.execute(ExecInput::default()); - runner.after_execution().await.expect("failed to run after execution hook"); + let input = ExecInput::default(); + runner.seed_execution(input).expect("failed to seed execution"); + let rx = runner.execute(input); + runner.consensus.update_tip(H256::from_low_u64_be(1)); + let result = rx.await.unwrap(); assert_matches!( - rx.await.unwrap(), + result, Ok(ExecOutput { done, reached_tip, stage_progress: out_stage_progress }) - if !done && !reached_tip && out_stage_progress == stage_progress + if !done && !reached_tip && out_stage_progress == 0 ); - assert!(runner.validate_execution().is_ok(), "validation failed"); + assert!(runner.validate_execution(input).is_ok(), "validation failed"); } #[tokio::test] - // Validate that all necessary tables are updated after the - // header download with some previous progress. - async fn execute_prev_progress() { - let mut runner = HeadersTestRunner::default(); - let (stage_progress, previous_stage) = (10000, 10241); + // Execute the stage with linear downloader + async fn execute_with_linear_downloader() { + let mut runner = HeadersTestRunner::with_linear_downloader(); + let (stage_progress, previous_stage) = (1000, 1200); let input = ExecInput { previous_stage: Some((PREV_STAGE_ID, previous_stage)), stage_progress: Some(stage_progress), }; runner.seed_execution(input).expect("failed to seed execution"); let rx = runner.execute(input); - runner.after_execution().await.expect("failed to run after execution hook"); + + // skip `after_execution` hook for linear downloader + let headers = runner.context.as_ref().unwrap(); + let tip = headers.last().unwrap(); + runner.consensus.update_tip(tip.hash()); + + let download_result = headers.clone(); + runner + .client + .on_header_request(1, |id, _| { + let response = download_result.clone().into_iter().map(|h| h.unseal()).collect(); + runner.client.send_header_response(id, response) + }) + .await; + assert_matches!( rx.await.unwrap(), Ok(ExecOutput { done, reached_tip, stage_progress }) - if done && reached_tip && stage_progress == stage_progress + if done && reached_tip && stage_progress == tip.number ); - assert!(runner.validate_execution().is_ok(), "validation failed"); + assert!(runner.validate_execution(input).is_ok(), "validation failed"); } - // TODO: - // #[tokio::test] - // // Execute the stage with linear downloader - // async fn execute_with_linear_downloader() { - // let mut runner = HeadersTestRunner::with_linear_downloader(); - // let (stage_progress, previous_stage) = (1000, 1200); - // let input = ExecInput { - // previous_stage: Some((PREV_STAGE_ID, previous_stage)), - // stage_progress: Some(stage_progress), - // }; - // runner.seed_execution(input).expect("failed to seed execution"); - // let rx = runner.execute(input); - - // // skip hook for linear downloader - // let headers = runner.context.as_ref().unwrap(); - // let tip = headers.first().unwrap(); - // runner.consensus.update_tip(tip.hash()); - - // // TODO: - // let mut download_result = headers.clone(); - // download_result.insert(0, headers.last().unwrap().clone()); - // runner - // .client - // .on_header_request(1, |id, _| { - // let response = download_result.clone().into_iter().map(|h| h.unseal()).collect(); - // runner.client.send_header_response(id, response) - // }) - // .await; - - // assert_matches!( - // rx.await.unwrap(), - // Ok(ExecOutput { done, reached_tip, stage_progress }) - // if done && reached_tip && stage_progress == tip.number - // ); - // assert!(runner.validate_execution().is_ok(), "validation failed"); - // } - - // TODO: - // #[tokio::test] - // // Check that unwind can remove headers across gaps - // async fn unwind_db_gaps() { - // let runner = HeadersTestRunner::default(); - // let head = gen_random_header(0, None); - // let first_range = gen_random_header_range(1..20, head.hash()); - // let second_range = gen_random_header_range(50..100, H256::zero()); - // runner.insert_header(&head).expect("failed to insert header"); - // runner - // .insert_headers(first_range.iter().chain(second_range.iter())) - // .expect("failed to insert headers"); - - // let unwind_to = 15; - // let input = UnwindInput { bad_block: None, stage_progress: unwind_to, unwind_to }; - // let rx = runner.unwind(input); - // assert_matches!( - // rx.await.unwrap(), - // Ok(UnwindOutput {stage_progress} ) if stage_progress == unwind_to - // ); - // assert!(runner.validate_unwind(input).is_ok(), "validation failed"); - // } - mod test_runner { use crate::{ stages::headers::HeaderStage, @@ -370,7 +317,7 @@ mod tests { let end = input.previous_stage.map(|(_, num)| num).unwrap_or_default() + 1; if end > start + 1 { let mut headers = gen_random_header_range(start + 1..end, head.hash()); - headers.reverse(); + headers.insert(0, head); self.context = Some(headers); } Ok(()) @@ -381,27 +328,28 @@ mod tests { ) -> Result<(), Box> { let (tip, headers) = match self.context { Some(ref headers) if headers.len() > 1 => { - // headers are in reverse - (headers.first().unwrap().hash(), headers.clone()) + (headers.last().unwrap().hash(), headers.clone()) } _ => (H256::from_low_u64_be(rand::random()), Vec::default()), }; self.consensus.update_tip(tip); - if !headers.is_empty() { - self.client - .send_header_response_delayed( - 0, - headers.iter().cloned().map(|h| h.unseal()).collect(), - 1, - ) - .await; - } + self.client + .send_header_response_delayed( + 0, + headers.iter().cloned().map(|h| h.unseal()).collect(), + 1, + ) + .await; Ok(()) } - fn validate_execution(&self) -> Result<(), Box> { + fn validate_execution( + &self, + _input: ExecInput, + ) -> Result<(), Box> { if let Some(ref headers) = self.context { - headers.iter().try_for_each(|h| self.validate_db_header(&h))?; + // skip head and validate each + headers.iter().skip(1).try_for_each(|h| self.validate_db_header(&h))?; } Ok(()) } @@ -546,11 +494,18 @@ mod tests { _: &ForkchoiceState, ) -> Result, DownloadError> { let stream = self.client.stream_headers().await; - let stream = stream.timeout(Duration::from_secs(3)); + let stream = stream.timeout(Duration::from_secs(1)); match Box::pin(stream).try_next().await { Ok(Some(res)) => { - Ok(res.headers.iter().map(|h| h.clone().seal()).collect::>()) + let mut headers = + res.headers.iter().map(|h| h.clone().seal()).collect::>(); + if !headers.is_empty() { + headers.sort_unstable_by_key(|h| h.number); + headers.remove(0); // remove head from response + headers.reverse(); + } + Ok(headers) } _ => Err(DownloadError::Timeout { request_id: 0 }), } diff --git a/crates/stages/src/stages/tx_index.rs b/crates/stages/src/stages/tx_index.rs index 05b45cb383f..808aed6ba5d 100644 --- a/crates/stages/src/stages/tx_index.rs +++ b/crates/stages/src/stages/tx_index.rs @@ -89,7 +89,7 @@ mod tests { use super::*; use crate::util::test_utils::{ stage_test_suite, ExecuteStageTestRunner, StageTestDB, StageTestRunner, - UnwindStageTestRunner, PREV_STAGE_ID, + UnwindStageTestRunner, }; use assert_matches::assert_matches; use reth_interfaces::{db::models::BlockNumHash, test_utils::gen_random_header_range}; @@ -97,83 +97,6 @@ mod tests { stage_test_suite!(TxIndexTestRunner); - #[tokio::test] - async fn execute_no_prev_tx_count() { - let runner = TxIndexTestRunner::default(); - let headers = gen_random_header_range(0..10, H256::zero()); - runner - .db() - .map_put::(&headers, |h| (h.number, h.hash())) - .expect("failed to insert"); - - let (head, tail) = (headers.first().unwrap(), headers.last().unwrap()); - let input = ExecInput { - previous_stage: Some((PREV_STAGE_ID, tail.number)), - stage_progress: Some(head.number), - }; - let rx = runner.execute(input); - assert_matches!( - rx.await.unwrap(), - Err(StageError::DatabaseIntegrity(DatabaseIntegrityError::CumulativeTxCount { .. })) - ); - } - - #[tokio::test] - async fn unwind_no_input() { - let runner = TxIndexTestRunner::default(); - let headers = gen_random_header_range(0..10, H256::zero()); - runner - .db() - .transform_append::(&headers, |prev, h| { - ( - BlockNumHash((h.number, h.hash())), - prev.unwrap_or_default() + (rand::random::() as u64), - ) - }) - .expect("failed to insert"); - - let rx = runner.unwind(UnwindInput::default()); - assert_matches!( - rx.await.unwrap(), - Ok(UnwindOutput { stage_progress }) if stage_progress == 0 - ); - runner - .db() - .check_no_entry_above::(0, |h| h.number()) - .expect("failed to check tx count"); - } - - #[tokio::test] - async fn unwind_with_db_gaps() { - let runner = TxIndexTestRunner::default(); - let first_range = gen_random_header_range(0..20, H256::zero()); - let second_range = gen_random_header_range(50..100, H256::zero()); - runner - .db() - .transform_append::( - &first_range.iter().chain(second_range.iter()).collect::>(), - |prev, h| { - ( - BlockNumHash((h.number, h.hash())), - prev.unwrap_or_default() + (rand::random::() as u64), - ) - }, - ) - .expect("failed to insert"); - - let unwind_to = 10; - let input = UnwindInput { unwind_to, ..Default::default() }; - let rx = runner.unwind(input); - assert_matches!( - rx.await.unwrap(), - Ok(UnwindOutput { stage_progress }) if stage_progress == unwind_to - ); - runner - .db() - .check_no_entry_above::(unwind_to, |h| h.number()) - .expect("failed to check tx count"); - } - #[derive(Default)] pub(crate) struct TxIndexTestRunner { db: StageTestDB, @@ -198,24 +121,60 @@ mod tests { ) -> Result<(), Box> { let pivot = input.stage_progress.unwrap_or_default(); let start = pivot.saturating_sub(100); - let end = input.previous_stage.as_ref().map(|(_, num)| *num).unwrap_or_default(); + let mut end = input.previous_stage.as_ref().map(|(_, num)| *num).unwrap_or_default(); + end += 2; // generate 2 additional headers to account for start header lookup let headers = gen_random_header_range(start..end, H256::zero()); - self.db() - .map_put::(&headers, |h| (h.number, h.hash()))?; + + let headers = + headers.into_iter().map(|h| (h, rand::random::())).collect::>(); + + self.db().map_put::(&headers, |(h, _)| { + (h.number, h.hash()) + })?; + self.db().map_put::(&headers, |(h, count)| { + (BlockNumHash((h.number, h.hash())), *count as u16) + })?; + + let slice_up_to = + std::cmp::min(pivot.saturating_sub(start) as usize, headers.len() - 1); self.db().transform_append::( - &headers[..=(pivot as usize)], - |prev, h| { - ( - BlockNumHash((h.number, h.hash())), - prev.unwrap_or_default() + (rand::random::() as u64), - ) + &headers[..=slice_up_to], + |prev, (h, count)| { + (BlockNumHash((h.number, h.hash())), prev.unwrap_or_default() + (*count as u64)) }, )?; + Ok(()) } - fn validate_execution(&self) -> Result<(), Box> { - // TODO: + fn validate_execution( + &self, + input: ExecInput, + ) -> Result<(), Box> { + let db = self.db().container(); + let tx = db.get(); + let (start, end) = ( + input.stage_progress.unwrap_or_default(), + input.previous_stage.as_ref().map(|(_, num)| *num).unwrap_or_default(), + ); + if start >= end { + return Ok(()) + } + + let start_hash = + tx.get::(start)?.expect("no canonical found"); + let mut tx_count_cursor = tx.cursor::()?; + let mut tx_count_walker = tx_count_cursor.walk((start, start_hash).into())?; + let mut count = tx_count_walker.next().unwrap()?.1; + let mut last_num = start; + while let Some(entry) = tx_count_walker.next() { + let (key, db_count) = entry?; + count += tx.get::(key)?.unwrap() as u64; + assert_eq!(db_count, count); + last_num = key.number(); + } + assert_eq!(last_num, end); + Ok(()) } } @@ -226,7 +185,6 @@ mod tests { input: UnwindInput, highest_entry: u64, ) -> Result<(), Box> { - // TODO: accept range let headers = gen_random_header_range(input.unwind_to..highest_entry, H256::zero()); self.db().transform_append::( &headers, diff --git a/crates/stages/src/util.rs b/crates/stages/src/util.rs index a9817964fe9..5bed834f0a0 100644 --- a/crates/stages/src/util.rs +++ b/crates/stages/src/util.rs @@ -181,6 +181,7 @@ pub(crate) mod test_utils { DBContainer::new(self.db.borrow()).expect("failed to create db container") } + /// Invoke a callback with transaction committing it afterwards fn commit(&self, f: F) -> Result<(), Error> where F: FnOnce(&mut Tx<'_, RW, WriteMap>) -> Result<(), Error>, @@ -192,19 +193,6 @@ pub(crate) mod test_utils { Ok(()) } - /// Put a single value into the table - pub(crate) fn put(&self, k: T::Key, v: T::Value) -> Result<(), Error> { - self.commit(|tx| tx.put::(k, v)) - } - - /// Delete a single value from the table - pub(crate) fn delete(&self, k: T::Key) -> Result<(), Error> { - self.commit(|tx| { - tx.delete::(k, None)?; - Ok(()) - }) - } - /// Map a collection of values and store them in the database. /// This function commits the transaction before exiting. /// @@ -323,7 +311,10 @@ pub(crate) mod test_utils { ) -> Result<(), Box>; /// Validate stage execution - fn validate_execution(&self) -> Result<(), Box>; + fn validate_execution( + &self, + input: ExecInput, + ) -> Result<(), Box>; /// Run [Stage::execute] and return a receiver for the result. fn execute(&self, input: ExecInput) -> oneshot::Receiver> { @@ -383,12 +374,32 @@ pub(crate) mod test_utils { // prev progress missing from the database. async fn execute_empty_db() { let runner = $runner::default(); - let rx = runner.execute(crate::stage::ExecInput::default()); + let input = crate::stage::ExecInput::default(); + let rx = runner.execute(input); assert_matches!( rx.await.unwrap(), Err(crate::error::StageError::DatabaseIntegrity(_)) ); - assert!(runner.validate_execution().is_ok(), "execution validation"); + assert!(runner.validate_execution(input).is_ok(), "execution validation"); + } + + #[tokio::test] + async fn execute_no_progress() { + let stage_progress = 1000; + let mut runner = $runner::default(); + let input = crate::stage::ExecInput { + previous_stage: Some((crate::util::test_utils::PREV_STAGE_ID, stage_progress)), + stage_progress: Some(stage_progress), + }; + runner.seed_execution(input).expect("failed to seed"); + let rx = runner.execute(input); + runner.after_execution().await.expect("failed to run after execution hook"); + assert_matches!( + rx.await.unwrap(), + Ok(ExecOutput { done, reached_tip, stage_progress }) + if done && reached_tip && stage_progress == stage_progress + ); + assert!(runner.validate_execution(input).is_ok(), "execution validation"); } #[tokio::test] @@ -407,7 +418,7 @@ pub(crate) mod test_utils { Ok(ExecOutput { done, reached_tip, stage_progress }) if done && reached_tip && stage_progress == previous_stage ); - assert!(runner.validate_execution().is_ok(), "execution validation"); + assert!(runner.validate_execution(input).is_ok(), "execution validation"); } #[tokio::test] From 640cad2cb6c5859bb7b6b15a6495044f4eee634e Mon Sep 17 00:00:00 2001 From: Roman Krasiuk Date: Wed, 16 Nov 2022 08:43:00 +0200 Subject: [PATCH 03/17] nit --- crates/stages/src/stages/headers.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/stages/src/stages/headers.rs b/crates/stages/src/stages/headers.rs index f8a93e118eb..c6fe3e56771 100644 --- a/crates/stages/src/stages/headers.rs +++ b/crates/stages/src/stages/headers.rs @@ -198,9 +198,8 @@ mod tests { runner.seed_execution(input).expect("failed to seed execution"); let rx = runner.execute(input); runner.consensus.update_tip(H256::from_low_u64_be(1)); - let result = rx.await.unwrap(); assert_matches!( - result, + rx.await.unwrap(), Ok(ExecOutput { done, reached_tip, stage_progress: out_stage_progress }) if !done && !reached_tip && out_stage_progress == 0 ); @@ -228,7 +227,7 @@ mod tests { runner .client .on_header_request(1, |id, _| { - let response = download_result.clone().into_iter().map(|h| h.unseal()).collect(); + let response = download_result.iter().map(|h| h.clone().unseal()).collect(); runner.client.send_header_response(id, response) }) .await; From 8c0222a3cc98d6b9d791f330a6207d14365c88da Mon Sep 17 00:00:00 2001 From: Roman Krasiuk Date: Wed, 16 Nov 2022 18:25:35 +0200 Subject: [PATCH 04/17] start revamping bodies testing --- crates/stages/src/stages/bodies.rs | 243 +++++++++++++++------------ crates/stages/src/stages/headers.rs | 130 +++++++------- crates/stages/src/stages/tx_index.rs | 70 ++++---- crates/stages/src/util.rs | 86 +++++----- 4 files changed, 283 insertions(+), 246 deletions(-) diff --git a/crates/stages/src/stages/bodies.rs b/crates/stages/src/stages/bodies.rs index 5a423f1718d..498034e9956 100644 --- a/crates/stages/src/stages/bodies.rs +++ b/crates/stages/src/stages/bodies.rs @@ -232,7 +232,7 @@ impl BodyStage { #[cfg(test)] mod tests { use super::*; - use crate::util::test_utils::StageTestRunner; + use crate::util::test_utils::{ExecuteStageTestRunner, StageTestRunner, UnwindStageTestRunner, PREV_STAGE_ID}; use assert_matches::assert_matches; use reth_eth_wire::BlockBody; use reth_interfaces::{ @@ -260,7 +260,7 @@ mod tests { async fn already_reached_target() { let runner = BodyTestRunner::new(TestBodyDownloader::default); let rx = runner.execute(ExecInput { - previous_stage: Some((StageId("Headers"), 100)), + previous_stage: Some((PREV_STAGE_ID, 100)), stage_progress: Some(100), }); assert_matches!( @@ -289,10 +289,11 @@ mod tests { .expect("Could not insert headers"); // Run the stage - let rx = runner.execute(ExecInput { - previous_stage: Some((StageId("Headers"), blocks.len() as BlockNumber)), + let input = ExecInput { + previous_stage: Some((PREV_STAGE_ID, blocks.len() as BlockNumber)), stage_progress: None, - }); + }; + let rx = runner.execute(input); // Check that we only synced around `batch_size` blocks even though the number of blocks // synced by the previous stage is higher @@ -301,9 +302,7 @@ mod tests { output, Ok(ExecOutput { stage_progress, reached_tip: true, done: false }) if stage_progress < 200 ); - runner - .validate_db_blocks(output.unwrap().stage_progress) - .expect("Written block data invalid"); + assert!(runner.validate_execution(input, output.ok()).is_ok(), "execution validation"); } /// Same as [partial_body_download] except the `batch_size` is not hit. @@ -325,10 +324,11 @@ mod tests { .expect("Could not insert headers"); // Run the stage - let rx = runner.execute(ExecInput { - previous_stage: Some((StageId("Headers"), blocks.len() as BlockNumber)), + let input = ExecInput { + previous_stage: Some((PREV_STAGE_ID, blocks.len() as BlockNumber)), stage_progress: None, - }); + }; + let rx = runner.execute(input); // Check that we synced all blocks successfully, even though our `batch_size` allows us to // sync more (if there were more headers) @@ -337,9 +337,7 @@ mod tests { output, Ok(ExecOutput { stage_progress: 20, reached_tip: true, done: true }) ); - runner - .validate_db_blocks(output.unwrap().stage_progress) - .expect("Written block data invalid"); + assert!(runner.validate_execution(input, output.ok()).is_ok(), "execution validation"); } /// Same as [full_body_download] except we have made progress before @@ -359,7 +357,7 @@ mod tests { // Run the stage let rx = runner.execute(ExecInput { - previous_stage: Some((StageId("Headers"), blocks.len() as BlockNumber)), + previous_stage: Some((PREV_STAGE_ID, blocks.len() as BlockNumber)), stage_progress: None, }); @@ -372,10 +370,11 @@ mod tests { let first_run_progress = first_run.unwrap().stage_progress; // Execute again on top of the previous run - let rx = runner.execute(ExecInput { - previous_stage: Some((StageId("Headers"), blocks.len() as BlockNumber)), + let input = ExecInput { + previous_stage: Some((PREV_STAGE_ID, blocks.len() as BlockNumber)), stage_progress: Some(first_run_progress), - }); + }; + let rx = runner.execute(input); // Check that we synced more blocks let output = rx.await.unwrap(); @@ -383,9 +382,7 @@ mod tests { output, Ok(ExecOutput { stage_progress, reached_tip: true, done: true }) if stage_progress > first_run_progress ); - runner - .validate_db_blocks(output.unwrap().stage_progress) - .expect("Written block data invalid"); + assert!(runner.validate_execution(input, output.ok()).is_ok(), "execution validation"); } /// Checks that the stage asks to unwind if pre-validation of the block fails. @@ -408,7 +405,7 @@ mod tests { // Run the stage let rx = runner.execute(ExecInput { - previous_stage: Some((StageId("Headers"), blocks.len() as BlockNumber)), + previous_stage: Some((PREV_STAGE_ID, blocks.len() as BlockNumber)), stage_progress: None, }); @@ -424,11 +421,13 @@ mod tests { async fn unwind_empty_db() { let unwind_to = 10; let runner = BodyTestRunner::new(TestBodyDownloader::default); - let rx = runner.unwind(UnwindInput { bad_block: None, stage_progress: 20, unwind_to }); + let input = UnwindInput { bad_block: None, stage_progress: 20, unwind_to }; + let rx = runner.unwind(input); assert_matches!( rx.await.unwrap(), Ok(UnwindOutput { stage_progress }) if stage_progress == unwind_to - ) + ); + assert!(runner.validate_unwind(input).is_ok(), "unwind validation"); } /// Checks that the stage unwinds correctly with data. @@ -450,10 +449,11 @@ mod tests { .expect("Could not insert headers"); // Run the stage - let rx = runner.execute(ExecInput { - previous_stage: Some((StageId("Headers"), blocks.len() as BlockNumber)), + let execute_input = ExecInput { + previous_stage: Some((PREV_STAGE_ID, blocks.len() as BlockNumber)), stage_progress: None, - }); + }; + let rx = runner.execute(execute_input); // Check that we synced all blocks successfully, even though our `batch_size` allows us to // sync more (if there were more headers) @@ -462,27 +462,19 @@ mod tests { output, Ok(ExecOutput { stage_progress: 20, reached_tip: true, done: true }) ); - let stage_progress = output.unwrap().stage_progress; - runner.validate_db_blocks(stage_progress).expect("Written block data invalid"); + let output = output.unwrap(); + assert!(runner.validate_execution(execute_input, Some(output.clone())).is_ok(), "execution validation"); // Unwind all of it let unwind_to = 1; - let rx = runner.unwind(UnwindInput { bad_block: None, stage_progress, unwind_to }); + let unwind_input = UnwindInput { bad_block: None, stage_progress: output.stage_progress, unwind_to }; + let rx = runner.unwind(unwind_input); assert_matches!( rx.await.unwrap(), Ok(UnwindOutput { stage_progress }) if stage_progress == 1 ); - let last_body = runner.last_body().expect("Could not read last body"); - let last_tx_id = last_body.base_tx_id + last_body.tx_amount; - runner - .db() - .check_no_entry_above::(unwind_to, |key| key.number()) - .expect("Did not unwind block bodies correctly."); - runner - .db() - .check_no_entry_above::(last_tx_id, |key| key) - .expect("Did not unwind transactions correctly.") + assert!(runner.validate_unwind(unwind_input).is_ok(), "unwind validation"); } /// Checks that the stage unwinds correctly, even if a transaction in a block is missing. @@ -505,7 +497,7 @@ mod tests { // Run the stage let rx = runner.execute(ExecInput { - previous_stage: Some((StageId("Headers"), blocks.len() as BlockNumber)), + previous_stage: Some((PREV_STAGE_ID, blocks.len() as BlockNumber)), stage_progress: None, }); @@ -520,38 +512,23 @@ mod tests { runner.validate_db_blocks(stage_progress).expect("Written block data invalid"); // Delete a transaction - { - let mut db = runner.db().container(); - let mut tx_cursor = db - .get_mut() - .cursor_mut::() - .expect("Could not get transaction cursor"); - tx_cursor - .last() - .expect("Could not read database") - .expect("Could not read last transaction"); - tx_cursor.delete_current().expect("Could not delete last transaction"); - db.commit().expect("Could not commit database"); - } + runner.db().commit(|tx| { + let mut tx_cursor = tx.cursor_mut::()?; + tx_cursor.last()?.expect("Could not read last transaction"); + tx_cursor.delete_current()?; + Ok(()) + }).expect("Could not delete a transaction"); // Unwind all of it let unwind_to = 1; - let rx = runner.unwind(UnwindInput { bad_block: None, stage_progress, unwind_to }); + let input = UnwindInput { bad_block: None, stage_progress, unwind_to }; + let rx = runner.unwind(input); assert_matches!( rx.await.unwrap(), Ok(UnwindOutput { stage_progress }) if stage_progress == 1 ); - let last_body = runner.last_body().expect("Could not read last body"); - let last_tx_id = last_body.base_tx_id + last_body.tx_amount; - runner - .db() - .check_no_entry_above::(unwind_to, |key| key.number()) - .expect("Did not unwind block bodies correctly."); - runner - .db() - .check_no_entry_above::(last_tx_id, |key| key) - .expect("Did not unwind transactions correctly.") + assert!(runner.validate_unwind(input).is_ok(), "unwind validation"); } /// Checks that the stage exits if the downloader times out @@ -574,7 +551,7 @@ mod tests { // Run the stage let rx = runner.execute(ExecInput { - previous_stage: Some((StageId("Headers"), 1)), + previous_stage: Some((PREV_STAGE_ID, 1)), stage_progress: None, }); @@ -585,7 +562,10 @@ mod tests { mod test_utils { use crate::{ stages::bodies::BodyStage, - util::test_utils::{StageTestDB, StageTestRunner}, + util::test_utils::{ + ExecuteStageTestRunner, StageTestDB, StageTestRunner, UnwindStageTestRunner, TestRunnerError, + }, + ExecInput, UnwindInput, ExecOutput, }; use assert_matches::assert_matches; use async_trait::async_trait; @@ -680,6 +660,61 @@ mod tests { } } + impl ExecuteStageTestRunner for BodyTestRunner + where + F: Fn() -> TestBodyDownloader, + { + type Seed = (); + + fn seed_execution( + &mut self, + input: ExecInput, + ) -> Result<(), TestRunnerError> { + self.insert_genesis()?; + // TODO: + // self + // .insert_headers(blocks.iter().map(|block| &block.header)) + // .expect("Could not insert headers"); + Ok(()) + } + + fn validate_execution( + &self, + input: ExecInput, + output: Option, + ) -> Result<(), TestRunnerError> { + if let Some(output) = output { + self.validate_db_blocks(output.stage_progress)?; + } + Ok(()) + } + } + + impl UnwindStageTestRunner for BodyTestRunner + where + F: Fn() -> TestBodyDownloader, + { + fn seed_unwind( + &mut self, + input: UnwindInput, + highest_entry: u64, + ) -> Result<(), TestRunnerError> { + unimplemented!() + } + + fn validate_unwind( + &self, + input: UnwindInput, + ) -> Result<(), TestRunnerError> { + self.db.check_no_entry_above::(input.unwind_to, |key| key.number())?; + if let Some(last_body) =self.last_body(){ + let last_tx_id = last_body.base_tx_id + last_body.tx_amount; + self.db.check_no_entry_above::(last_tx_id, |key| key)?; + } + Ok(()) + } + } + impl BodyTestRunner where F: Fn() -> TestBodyDownloader, @@ -690,13 +725,12 @@ mod tests { /// same hash. pub(crate) fn insert_genesis(&self) -> Result<(), db::Error> { self.insert_header(&SealedHeader::new(Header::default(), GENESIS_HASH))?; - let mut db = self.db.container(); - let tx = db.get_mut(); - tx.put::( - (0, GENESIS_HASH).into(), - StoredBlockBody { base_tx_id: 0, tx_amount: 0, ommers: vec![] }, - )?; - db.commit()?; + self.db.commit(|tx| { + tx.put::( + (0, GENESIS_HASH).into(), + StoredBlockBody { base_tx_id: 0, tx_amount: 0, ommers: vec![] }, + ) + })?; Ok(()) } @@ -707,6 +741,7 @@ mod tests { } /// Insert headers into tables + /// TODO: move to common inserter pub(crate) fn insert_headers<'a, I>(&self, headers: I) -> Result<(), db::Error> where I: Iterator, @@ -733,9 +768,10 @@ mod tests { } pub(crate) fn last_body(&self) -> Option { - Some( - self.db.container().get().cursor::().ok()?.last().ok()??.1, - ) + self.db + .query(|tx| Ok(tx.cursor::()?.last()?.map(|e| e.1))) + .ok() + .flatten() } /// Validate that the inserted block data is valid @@ -743,35 +779,34 @@ mod tests { &self, highest_block: BlockNumber, ) -> Result<(), db::Error> { - let db = self.db.container(); - let tx = db.get(); - - let mut block_body_cursor = tx.cursor::()?; - let mut transaction_cursor = tx.cursor::()?; - - let mut entry = block_body_cursor.first()?; - let mut prev_max_tx_id = 0; - while let Some((key, body)) = entry { - assert!( - key.number() <= highest_block, - "We wrote a block body outside of our synced range. Found block with number {}, highest block according to stage is {}", - key.number(), highest_block - ); - - assert!(prev_max_tx_id == body.base_tx_id, "Transaction IDs are malformed."); - for num in 0..body.tx_amount { - let tx_id = body.base_tx_id + num; - assert_matches!( - transaction_cursor.seek_exact(tx_id), - Ok(Some(_)), - "A transaction is missing." + self.db.query(|tx| { + let mut block_body_cursor = tx.cursor::()?; + let mut transaction_cursor = tx.cursor::()?; + + let mut entry = block_body_cursor.first()?; + let mut prev_max_tx_id = 0; + while let Some((key, body)) = entry { + assert!( + key.number() <= highest_block, + "We wrote a block body outside of our synced range. Found block with number {}, highest block according to stage is {}", + key.number(), highest_block ); + + assert!(prev_max_tx_id == body.base_tx_id, "Transaction IDs are malformed."); + for num in 0..body.tx_amount { + let tx_id = body.base_tx_id + num; + assert_matches!( + transaction_cursor.seek_exact(tx_id), + Ok(Some(_)), + "A transaction is missing." + ); + } + prev_max_tx_id = body.base_tx_id + body.tx_amount; + entry = block_body_cursor.next()?; } - prev_max_tx_id = body.base_tx_id + body.tx_amount; - entry = block_body_cursor.next()?; - } - - Ok(()) + + Ok(()) + }) } } diff --git a/crates/stages/src/stages/headers.rs b/crates/stages/src/stages/headers.rs index 0a7ec70e9a8..0ea0c86dea5 100644 --- a/crates/stages/src/stages/headers.rs +++ b/crates/stages/src/stages/headers.rs @@ -198,6 +198,8 @@ mod tests { stage_test_suite!(HeadersTestRunner); + // TODO: test consensus propagation error + /// Check that the execution errors on empty database or /// prev progress missing from the database. #[tokio::test] @@ -208,12 +210,12 @@ mod tests { runner.seed_execution(input).expect("failed to seed execution"); let rx = runner.execute(input); runner.consensus.update_tip(H256::from_low_u64_be(1)); + let result = rx.await.unwrap(); assert_matches!( - rx.await.unwrap(), - Ok(ExecOutput { done, reached_tip, stage_progress: out_stage_progress }) - if !done && !reached_tip && out_stage_progress == 0 + result, + Ok(ExecOutput { done: false, reached_tip: false, stage_progress: 0 }) ); - assert!(runner.validate_execution(input).is_ok(), "validation failed"); + assert!(runner.validate_execution(input, result.ok()).is_ok(), "validation failed"); } /// Execute the stage with linear downloader @@ -225,11 +227,10 @@ mod tests { previous_stage: Some((PREV_STAGE_ID, previous_stage)), stage_progress: Some(stage_progress), }; - runner.seed_execution(input).expect("failed to seed execution"); + let headers = runner.seed_execution(input).expect("failed to seed execution"); let rx = runner.execute(input); // skip `after_execution` hook for linear downloader - let headers = runner.context.as_ref().unwrap(); let tip = headers.last().unwrap(); runner.consensus.update_tip(tip.hash()); @@ -242,21 +243,22 @@ mod tests { }) .await; + let result = rx.await.unwrap(); assert_matches!( - rx.await.unwrap(), - Ok(ExecOutput { done, reached_tip, stage_progress }) - if done && reached_tip && stage_progress == tip.number + result, + Ok(ExecOutput { done: true, reached_tip: true, stage_progress }) if stage_progress == tip.number ); - assert!(runner.validate_execution(input).is_ok(), "validation failed"); + assert!(runner.validate_execution(input, result.ok()).is_ok(), "validation failed"); } mod test_runner { use crate::{ stages::headers::HeaderStage, util::test_utils::{ - ExecuteStageTestRunner, StageTestDB, StageTestRunner, UnwindStageTestRunner, + ExecuteStageTestRunner, StageTestDB, StageTestRunner, TestRunnerError, + UnwindStageTestRunner, }, - ExecInput, UnwindInput, + ExecInput, ExecOutput, UnwindInput, }; use reth_headers_downloaders::linear::{LinearDownloadBuilder, LinearDownloader}; use reth_interfaces::{ @@ -273,7 +275,6 @@ mod tests { pub(crate) struct HeadersTestRunner { pub(crate) consensus: Arc, pub(crate) client: Arc, - pub(crate) context: Option>, downloader: Arc, db: StageTestDB, } @@ -286,7 +287,6 @@ mod tests { consensus: Arc::new(TestConsensus::default()), downloader: Arc::new(TestHeaderDownloader::new(client)), db: StageTestDB::default(), - context: None, } } } @@ -309,38 +309,36 @@ mod tests { #[async_trait::async_trait] impl ExecuteStageTestRunner for HeadersTestRunner { - fn seed_execution( - &mut self, - input: ExecInput, - ) -> Result<(), Box> { + type Seed = Vec; + + fn seed_execution(&mut self, input: ExecInput) -> Result { let start = input.stage_progress.unwrap_or_default(); let head = random_header(start, None); self.insert_header(&head)?; // use previous progress as seed size let end = input.previous_stage.map(|(_, num)| num).unwrap_or_default() + 1; - if end > start + 1 { - let mut headers = random_header_range(start + 1..end, head.hash()); - headers.insert(0, head); - self.context = Some(headers); + + if start + 1 >= end { + return Ok(Vec::default()) } - Ok(()) + + let mut headers = random_header_range(start + 1..end, head.hash()); + headers.insert(0, head); + Ok(headers) } - async fn after_execution( - &self, - ) -> Result<(), Box> { - let (tip, headers) = match self.context { - Some(ref headers) if headers.len() > 1 => { - (headers.last().unwrap().hash(), headers.clone()) - } - _ => (H256::from_low_u64_be(rand::random()), Vec::default()), + async fn after_execution(&self, headers: Self::Seed) -> Result<(), TestRunnerError> { + let tip = if !headers.is_empty() { + headers.last().unwrap().hash() + } else { + H256::from_low_u64_be(rand::random()) }; self.consensus.update_tip(tip); self.client .send_header_response_delayed( 0, - headers.iter().cloned().map(|h| h.unseal()).collect(), + headers.into_iter().map(|h| h.unseal()).collect(), 1, ) .await; @@ -350,11 +348,13 @@ mod tests { fn validate_execution( &self, _input: ExecInput, - ) -> Result<(), Box> { - if let Some(ref headers) = self.context { - // skip head and validate each - headers.iter().skip(1).try_for_each(|h| self.validate_db_header(&h))?; - } + _output: Option, + ) -> Result<(), TestRunnerError> { + // TODO: refine + // if let Some(ref headers) = self.context { + // // skip head and validate each + // headers.iter().skip(1).try_for_each(|h| self.validate_db_header(&h))?; + // } Ok(()) } } @@ -364,17 +364,14 @@ mod tests { &mut self, input: UnwindInput, highest_entry: u64, - ) -> Result<(), Box> { + ) -> Result<(), TestRunnerError> { let lowest_entry = input.unwind_to.saturating_sub(100); let headers = random_header_range(lowest_entry..highest_entry, H256::zero()); self.insert_headers(headers.iter())?; Ok(()) } - fn validate_unwind( - &self, - input: UnwindInput, - ) -> Result<(), Box> { + fn validate_unwind(&self, input: UnwindInput) -> Result<(), TestRunnerError> { let unwind_to = input.unwind_to; self.db.check_no_entry_above_by_value::( unwind_to, @@ -397,7 +394,7 @@ mod tests { let downloader = Arc::new( LinearDownloadBuilder::default().build(consensus.clone(), client.clone()), ); - Self { client, consensus, downloader, db: StageTestDB::default(), context: None } + Self { client, consensus, downloader, db: StageTestDB::default() } } } @@ -438,30 +435,31 @@ mod tests { &self, header: &SealedHeader, ) -> Result<(), db::Error> { - let db = self.db.container(); - let tx = db.get(); - let key: BlockNumHash = (header.number, header.hash()).into(); - - let db_number = tx.get::(header.hash())?; - assert_eq!(db_number, Some(header.number)); - - let db_header = tx.get::(key)?; - assert_eq!(db_header, Some(header.clone().unseal())); - - let db_canonical_header = tx.get::(header.number)?; - assert_eq!(db_canonical_header, Some(header.hash())); - - if header.number != 0 { - let parent_key: BlockNumHash = (header.number - 1, header.parent_hash).into(); - let parent_td = tx.get::(parent_key)?; - let td = U256::from_big_endian(&tx.get::(key)?.unwrap()); - assert_eq!( - parent_td.map(|td| U256::from_big_endian(&td) + header.difficulty), - Some(td) - ); - } + self.db.query(|tx| { + let key: BlockNumHash = (header.number, header.hash()).into(); + + let db_number = tx.get::(header.hash())?; + assert_eq!(db_number, Some(header.number)); + + let db_header = tx.get::(key)?; + assert_eq!(db_header, Some(header.clone().unseal())); + + let db_canonical_header = tx.get::(header.number)?; + assert_eq!(db_canonical_header, Some(header.hash())); + + if header.number != 0 { + let parent_key: BlockNumHash = + (header.number - 1, header.parent_hash).into(); + let parent_td = tx.get::(parent_key)?; + let td = U256::from_big_endian(&tx.get::(key)?.unwrap()); + assert_eq!( + parent_td.map(|td| U256::from_big_endian(&td) + header.difficulty), + Some(td) + ); + } - Ok(()) + Ok(()) + }) } } } diff --git a/crates/stages/src/stages/tx_index.rs b/crates/stages/src/stages/tx_index.rs index 2e8fd2c2595..fd4ac04f2e4 100644 --- a/crates/stages/src/stages/tx_index.rs +++ b/crates/stages/src/stages/tx_index.rs @@ -88,7 +88,7 @@ impl Stage for TxIndex { mod tests { use super::*; use crate::util::test_utils::{ - stage_test_suite, ExecuteStageTestRunner, StageTestDB, StageTestRunner, + stage_test_suite, ExecuteStageTestRunner, StageTestDB, StageTestRunner, TestRunnerError, UnwindStageTestRunner, }; use assert_matches::assert_matches; @@ -96,7 +96,7 @@ mod tests { db::models::{BlockNumHash, StoredBlockBody}, test_utils::generators::random_header_range, }; - use reth_primitives::H256; + use reth_primitives::{SealedHeader, H256}; stage_test_suite!(TxIndexTestRunner); @@ -118,10 +118,9 @@ mod tests { } impl ExecuteStageTestRunner for TxIndexTestRunner { - fn seed_execution( - &mut self, - input: ExecInput, - ) -> Result<(), Box> { + type Seed = (); + + fn seed_execution(&mut self, input: ExecInput) -> Result { let pivot = input.stage_progress.unwrap_or_default(); let start = pivot.saturating_sub(100); let mut end = input.previous_stage.as_ref().map(|(_, num)| *num).unwrap_or_default(); @@ -156,31 +155,33 @@ mod tests { fn validate_execution( &self, input: ExecInput, - ) -> Result<(), Box> { - let db = self.db.container(); - let tx = db.get(); - let (start, end) = ( - input.stage_progress.unwrap_or_default(), - input.previous_stage.as_ref().map(|(_, num)| *num).unwrap_or_default(), - ); - if start >= end { - return Ok(()) - } - - let start_hash = - tx.get::(start)?.expect("no canonical found"); - let mut tx_count_cursor = tx.cursor::()?; - let mut tx_count_walker = tx_count_cursor.walk((start, start_hash).into())?; - let mut count = tx_count_walker.next().unwrap()?.1; - let mut last_num = start; - while let Some(entry) = tx_count_walker.next() { - let (key, db_count) = entry?; - count += tx.get::(key)?.unwrap().tx_amount as u64; - assert_eq!(db_count, count); - last_num = key.number(); - } - assert_eq!(last_num, end); - + output: Option, + ) -> Result<(), TestRunnerError> { + self.db.query(|tx| { + let (start, end) = ( + input.stage_progress.unwrap_or_default(), + input.previous_stage.as_ref().map(|(_, num)| *num).unwrap_or_default(), + ); + if start >= end { + return Ok(()) + } + + let start_hash = + tx.get::(start)?.expect("no canonical found"); + let mut tx_count_cursor = tx.cursor::()?; + let mut tx_count_walker = tx_count_cursor.walk((start, start_hash).into())?; + let mut count = tx_count_walker.next().unwrap()?.1; + let mut last_num = start; + while let Some(entry) = tx_count_walker.next() { + let (key, db_count) = entry?; + count += tx.get::(key)?.unwrap().tx_amount as u64; + assert_eq!(db_count, count); + last_num = key.number(); + } + assert_eq!(last_num, end); + + Ok(()) + })?; Ok(()) } } @@ -190,7 +191,7 @@ mod tests { &mut self, input: UnwindInput, highest_entry: u64, - ) -> Result<(), Box> { + ) -> Result<(), TestRunnerError> { let headers = random_header_range(input.unwind_to..highest_entry, H256::zero()); self.db.transform_append::(&headers, |prev, h| { ( @@ -201,10 +202,7 @@ mod tests { Ok(()) } - fn validate_unwind( - &self, - input: UnwindInput, - ) -> Result<(), Box> { + fn validate_unwind(&self, input: UnwindInput) -> Result<(), TestRunnerError> { self.db.check_no_entry_above::(input.unwind_to, |h| { h.number() })?; diff --git a/crates/stages/src/util.rs b/crates/stages/src/util.rs index 5a2841d653e..6c5497385d3 100644 --- a/crates/stages/src/util.rs +++ b/crates/stages/src/util.rs @@ -142,7 +142,7 @@ pub(crate) mod test_utils { kv::{test_utils::create_test_db, tx::Tx, Env, EnvKind}, mdbx::{WriteMap, RW}, }; - use reth_interfaces::db::{DBContainer, DbCursorRO, DbCursorRW, DbTx, DbTxMut, Error, Table}; + use reth_interfaces::db::{self, DBContainer, DbCursorRO, DbCursorRW, DbTx, DbTxMut, Table}; use reth_primitives::BlockNumber; use std::{borrow::Borrow, sync::Arc}; use tokio::sync::oneshot; @@ -171,20 +171,20 @@ pub(crate) mod test_utils { } impl StageTestDB { + /// Return a database wrapped in [DBContainer]. + fn container(&self) -> DBContainer<'_, Env> { + DBContainer::new(self.db.borrow()).expect("failed to create db container") + } + /// Get a pointer to an internal database. pub(crate) fn inner(&self) -> Arc> { self.db.clone() } - /// Return a database wrapped in [DBContainer]. - pub(crate) fn container(&self) -> DBContainer<'_, Env> { - DBContainer::new(self.db.borrow()).expect("failed to create db container") - } - /// Invoke a callback with transaction committing it afterwards - fn commit(&self, f: F) -> Result<(), Error> + pub(crate) fn commit(&self, f: F) -> Result<(), db::Error> where - F: FnOnce(&mut Tx<'_, RW, WriteMap>) -> Result<(), Error>, + F: FnOnce(&mut Tx<'_, RW, WriteMap>) -> Result<(), db::Error>, { let mut db = self.container(); let tx = db.get_mut(); @@ -193,10 +193,10 @@ pub(crate) mod test_utils { Ok(()) } - /// Invoke a callback with transaction - fn query(&self, f: F) -> Result + /// Invoke a callback with a read transaction + pub(crate) fn query(&self, f: F) -> Result where - F: FnOnce(&Tx<'_, RW, WriteMap>) -> Result, + F: FnOnce(&Tx<'_, RW, WriteMap>) -> Result, { f(self.container().get()) } @@ -208,7 +208,7 @@ pub(crate) mod test_utils { /// let db = StageTestDB::default(); /// db.map_put::(&items, |item| item)?; /// ``` - pub(crate) fn map_put(&self, values: &[S], mut map: F) -> Result<(), Error> + pub(crate) fn map_put(&self, values: &[S], mut map: F) -> Result<(), db::Error> where T: Table, S: Clone, @@ -235,7 +235,7 @@ pub(crate) mod test_utils { &self, values: &[S], mut transform: F, - ) -> Result<(), Error> + ) -> Result<(), db::Error> where T: Table, ::Value: Clone, @@ -259,7 +259,7 @@ pub(crate) mod test_utils { &self, block: BlockNumber, mut selector: F, - ) -> Result<(), Error> + ) -> Result<(), db::Error> where T: Table, F: FnMut(T::Key) -> BlockNumber, @@ -279,7 +279,7 @@ pub(crate) mod test_utils { &self, block: BlockNumber, mut selector: F, - ) -> Result<(), Error> + ) -> Result<(), db::Error> where T: Table, F: FnMut(T::Value) -> BlockNumber, @@ -294,6 +294,14 @@ pub(crate) mod test_utils { } } + #[derive(thiserror::Error, Debug)] + pub(crate) enum TestRunnerError { + #[error("Database error occured.")] + Database(#[from] db::Error), + #[error("Internal runner error occured.")] + Internal(#[from] Box), + } + /// A generic test runner for stages. #[async_trait::async_trait] pub(crate) trait StageTestRunner { @@ -308,17 +316,17 @@ pub(crate) mod test_utils { #[async_trait::async_trait] pub(crate) trait ExecuteStageTestRunner: StageTestRunner { + type Seed: Send + Sync; + /// Seed database for stage execution - fn seed_execution( - &mut self, - input: ExecInput, - ) -> Result<(), Box>; + fn seed_execution(&mut self, input: ExecInput) -> Result; /// Validate stage execution fn validate_execution( &self, input: ExecInput, - ) -> Result<(), Box>; + output: Option, + ) -> Result<(), TestRunnerError>; /// Run [Stage::execute] and return a receiver for the result. fn execute(&self, input: ExecInput) -> oneshot::Receiver> { @@ -334,7 +342,7 @@ pub(crate) mod test_utils { } /// Run a hook after [Stage::execute]. Required for Headers & Bodies stages. - async fn after_execution(&self) -> Result<(), Box> { + async fn after_execution(&self, seed: Self::Seed) -> Result<(), TestRunnerError> { Ok(()) } } @@ -345,13 +353,10 @@ pub(crate) mod test_utils { &mut self, input: UnwindInput, highest_entry: u64, - ) -> Result<(), Box>; + ) -> Result<(), TestRunnerError>; /// Validate the unwind - fn validate_unwind( - &self, - input: UnwindInput, - ) -> Result<(), Box>; + fn validate_unwind(&self, input: UnwindInput) -> Result<(), TestRunnerError>; /// Run [Stage::unwind] and return a receiver for the result. fn unwind( @@ -373,37 +378,37 @@ pub(crate) mod test_utils { macro_rules! stage_test_suite { ($runner:ident) => { + /// Check that the execution is short-circuited if the database is empty. #[tokio::test] - // Check that the execution errors on empty database or - // prev progress missing from the database. async fn execute_empty_db() { let runner = $runner::default(); let input = crate::stage::ExecInput::default(); - let rx = runner.execute(input); + let result = runner.execute(input).await.unwrap(); assert_matches!( - rx.await.unwrap(), + result, Err(crate::error::StageError::DatabaseIntegrity(_)) ); - assert!(runner.validate_execution(input).is_ok(), "execution validation"); + assert!(runner.validate_execution(input, result.ok()).is_ok(), "execution validation"); } #[tokio::test] - async fn execute_no_progress() { + async fn execute_already_reached_target() { let stage_progress = 1000; let mut runner = $runner::default(); let input = crate::stage::ExecInput { previous_stage: Some((crate::util::test_utils::PREV_STAGE_ID, stage_progress)), stage_progress: Some(stage_progress), }; - runner.seed_execution(input).expect("failed to seed"); + let seed = runner.seed_execution(input).expect("failed to seed"); let rx = runner.execute(input); - runner.after_execution().await.expect("failed to run after execution hook"); + runner.after_execution(seed).await.expect("failed to run after execution hook"); + let result = rx.await.unwrap(); assert_matches!( - rx.await.unwrap(), + result, Ok(ExecOutput { done, reached_tip, stage_progress }) if done && reached_tip && stage_progress == stage_progress ); - assert!(runner.validate_execution(input).is_ok(), "execution validation"); + assert!(runner.validate_execution(input, result.ok()).is_ok(), "execution validation"); } #[tokio::test] @@ -414,15 +419,16 @@ pub(crate) mod test_utils { previous_stage: Some((crate::util::test_utils::PREV_STAGE_ID, previous_stage)), stage_progress: Some(stage_progress), }; - runner.seed_execution(input).expect("failed to seed"); + let seed = runner.seed_execution(input).expect("failed to seed"); let rx = runner.execute(input); - runner.after_execution().await.expect("failed to run after execution hook"); + runner.after_execution(seed).await.expect("failed to run after execution hook"); + let result = rx.await.unwrap(); assert_matches!( - rx.await.unwrap(), + result, Ok(ExecOutput { done, reached_tip, stage_progress }) if done && reached_tip && stage_progress == previous_stage ); - assert!(runner.validate_execution(input).is_ok(), "execution validation"); + assert!(runner.validate_execution(input, result.ok()).is_ok(), "execution validation"); } #[tokio::test] From fac647c602af304f03399f39100a8dfbbec488fa Mon Sep 17 00:00:00 2001 From: Roman Krasiuk Date: Thu, 17 Nov 2022 15:16:43 +0200 Subject: [PATCH 05/17] revamp body testing --- crates/interfaces/src/test_utils/headers.rs | 40 +- crates/stages/src/lib.rs | 3 + crates/stages/src/stages/bodies.rs | 401 +++++++------------- crates/stages/src/stages/headers.rs | 147 +++---- crates/stages/src/stages/tx_index.rs | 21 +- crates/stages/src/test_utils/macros.rs | 106 ++++++ crates/stages/src/test_utils/mod.rs | 14 + crates/stages/src/test_utils/runner.rs | 81 ++++ crates/stages/src/test_utils/stage_db.rs | 149 ++++++++ crates/stages/src/util.rs | 329 ---------------- 10 files changed, 599 insertions(+), 692 deletions(-) create mode 100644 crates/stages/src/test_utils/macros.rs create mode 100644 crates/stages/src/test_utils/mod.rs create mode 100644 crates/stages/src/test_utils/runner.rs create mode 100644 crates/stages/src/test_utils/stage_db.rs diff --git a/crates/interfaces/src/test_utils/headers.rs b/crates/interfaces/src/test_utils/headers.rs index 84ed8d79c92..4945627e379 100644 --- a/crates/interfaces/src/test_utils/headers.rs +++ b/crates/interfaces/src/test_utils/headers.rs @@ -1,6 +1,6 @@ //! Testing support for headers related interfaces. use crate::{ - consensus::{self, Consensus, Error}, + consensus::{self, Consensus}, p2p::headers::{ client::{HeadersClient, HeadersRequest, HeadersResponse, HeadersStream}, downloader::HeaderDownloader, @@ -9,7 +9,11 @@ use crate::{ }; use reth_primitives::{BlockLocked, Header, SealedHeader, H256, H512}; use reth_rpc_types::engine::ForkchoiceState; -use std::{collections::HashSet, sync::Arc, time::Duration}; +use std::{ + collections::HashSet, + sync::{Arc, Mutex, MutexGuard}, + time::Duration, +}; use tokio::sync::{broadcast, mpsc, watch}; use tokio_stream::{wrappers::BroadcastStream, StreamExt}; @@ -17,12 +21,13 @@ use tokio_stream::{wrappers::BroadcastStream, StreamExt}; #[derive(Debug)] pub struct TestHeaderDownloader { client: Arc, + consensus: Arc, } impl TestHeaderDownloader { /// Instantiates the downloader with the mock responses - pub fn new(client: Arc) -> Self { - Self { client } + pub fn new(client: Arc, consensus: Arc) -> Self { + Self { client, consensus } } } @@ -36,7 +41,7 @@ impl HeaderDownloader for TestHeaderDownloader { } fn consensus(&self) -> &Self::Consensus { - unimplemented!() + &self.consensus } fn client(&self) -> &Self::Client { @@ -48,6 +53,12 @@ impl HeaderDownloader for TestHeaderDownloader { _: &SealedHeader, _: &ForkchoiceState, ) -> Result, DownloadError> { + // call consensus stub first. fails if the flag is set + let empty = SealedHeader::default(); + self.consensus + .validate_header(&empty, &empty) + .map_err(|error| DownloadError::HeaderValidation { hash: empty.hash(), error })?; + let stream = self.client.stream_headers().await; let stream = stream.timeout(Duration::from_secs(1)); @@ -139,7 +150,7 @@ pub struct TestConsensus { /// Watcher over the forkchoice state channel: (watch::Sender, watch::Receiver), /// Flag whether the header validation should purposefully fail - fail_validation: bool, + fail_validation: Mutex, } impl Default for TestConsensus { @@ -150,7 +161,7 @@ impl Default for TestConsensus { finalized_block_hash: H256::zero(), safe_block_hash: H256::zero(), }), - fail_validation: false, + fail_validation: Mutex::new(false), } } } @@ -166,9 +177,14 @@ impl TestConsensus { self.channel.0.send(state).expect("updating fork choice state failed"); } + /// Acquire lock on failed validation flag + pub fn fail_validation(&self) -> MutexGuard<'_, bool> { + self.fail_validation.lock().expect("failed to acquite consensus mutex") + } + /// Update the validation flag - pub fn set_fail_validation(&mut self, val: bool) { - self.fail_validation = val; + pub fn set_fail_validation(&self, val: bool) { + *self.fail_validation() = val; } } @@ -183,15 +199,15 @@ impl Consensus for TestConsensus { _header: &SealedHeader, _parent: &SealedHeader, ) -> Result<(), consensus::Error> { - if self.fail_validation { + if *self.fail_validation() { Err(consensus::Error::BaseFeeMissing) } else { Ok(()) } } - fn pre_validate_block(&self, _block: &BlockLocked) -> Result<(), Error> { - if self.fail_validation { + fn pre_validate_block(&self, _block: &BlockLocked) -> Result<(), consensus::Error> { + if *self.fail_validation() { Err(consensus::Error::BaseFeeMissing) } else { Ok(()) diff --git a/crates/stages/src/lib.rs b/crates/stages/src/lib.rs index 697e2f525f1..b44b183ce64 100644 --- a/crates/stages/src/lib.rs +++ b/crates/stages/src/lib.rs @@ -20,6 +20,9 @@ mod pipeline; mod stage; mod util; +#[cfg(test)] +mod test_utils; + /// Implementations of stages. pub mod stages; diff --git a/crates/stages/src/stages/bodies.rs b/crates/stages/src/stages/bodies.rs index 498034e9956..c349d0ecc4d 100644 --- a/crates/stages/src/stages/bodies.rs +++ b/crates/stages/src/stages/bodies.rs @@ -15,7 +15,7 @@ use reth_primitives::{ proofs::{EMPTY_LIST_HASH, EMPTY_ROOT}, BlockLocked, BlockNumber, SealedHeader, H256, }; -use std::fmt::Debug; +use std::{fmt::Debug, sync::Arc}; use tracing::warn; const BODIES: StageId = StageId("Bodies"); @@ -51,9 +51,9 @@ const BODIES: StageId = StageId("Bodies"); #[derive(Debug)] pub struct BodyStage { /// The body downloader. - pub downloader: D, + pub downloader: Arc, /// The consensus engine. - pub consensus: C, + pub consensus: Arc, /// The maximum amount of block bodies to process in one stage execution. /// /// Smaller batch sizes result in less memory usage, but more disk I/O. Larger batch sizes @@ -232,67 +232,47 @@ impl BodyStage { #[cfg(test)] mod tests { use super::*; - use crate::util::test_utils::{ExecuteStageTestRunner, StageTestRunner, UnwindStageTestRunner, PREV_STAGE_ID}; + use crate::test_utils::{ExecuteStageTestRunner, StageTestRunner, UnwindStageTestRunner, PREV_STAGE_ID, stage_test_suite}; use assert_matches::assert_matches; - use reth_eth_wire::BlockBody; use reth_interfaces::{ consensus, p2p::bodies::error::DownloadError, - test_utils::generators::{random_block, random_block_range}, }; - use reth_primitives::{BlockNumber, H256}; use std::collections::HashMap; use test_utils::*; - /// Check that the execution is short-circuited if the database is empty. - #[tokio::test] - async fn empty_db() { - let runner = BodyTestRunner::new(TestBodyDownloader::default); - let rx = runner.execute(ExecInput::default()); - assert_matches!( - rx.await.unwrap(), - Ok(ExecOutput { stage_progress: 0, reached_tip: true, done: true }) - ) - } + stage_test_suite!(BodyTestRunner); - /// Check that the execution is short-circuited if the target was already reached. - #[tokio::test] - async fn already_reached_target() { - let runner = BodyTestRunner::new(TestBodyDownloader::default); - let rx = runner.execute(ExecInput { - previous_stage: Some((PREV_STAGE_ID, 100)), - stage_progress: Some(100), - }); - assert_matches!( - rx.await.unwrap(), - Ok(ExecOutput { stage_progress: 100, reached_tip: true, done: true }) - ) - } + /// Check that the execution is short-circuited if the database is empty. + // #[tokio::test] + // TODO: + // async fn empty_db() { + // let runner = BodyTestRunner::new(TestBodyDownloader::default); + // let rx = runner.execute(ExecInput::default()); + // assert_matches!( + // rx.await.unwrap(), + // Ok(ExecOutput { stage_progress: 0, reached_tip: true, done: true }) + // ) + // } /// Checks that the stage downloads at most `batch_size` blocks. #[tokio::test] async fn partial_body_download() { - // Generate blocks - let blocks = random_block_range(1..200, GENESIS_HASH); - let bodies: HashMap> = - blocks.iter().map(body_by_hash).collect(); - let mut runner = BodyTestRunner::new(|| TestBodyDownloader::new(bodies.clone())); + let (stage_progress, previous_stage) = (1, 200); + + // Set up test runner + let mut runner = BodyTestRunner::default(); + let input = ExecInput { + previous_stage: Some((PREV_STAGE_ID, previous_stage)), + stage_progress: Some(stage_progress), + }; + runner.seed_execution(input).expect("failed to seed execution"); // Set the batch size (max we sync per stage execution) to less than the number of blocks // the previous stage synced (10 vs 20) runner.set_batch_size(10); - // Insert required state - runner.insert_genesis().expect("Could not insert genesis block"); - runner - .insert_headers(blocks.iter().map(|block| &block.header)) - .expect("Could not insert headers"); - // Run the stage - let input = ExecInput { - previous_stage: Some((PREV_STAGE_ID, blocks.len() as BlockNumber)), - stage_progress: None, - }; let rx = runner.execute(input); // Check that we only synced around `batch_size` blocks even though the number of blocks @@ -308,26 +288,20 @@ mod tests { /// Same as [partial_body_download] except the `batch_size` is not hit. #[tokio::test] async fn full_body_download() { - // Generate blocks #1-20 - let blocks = random_block_range(1..21, GENESIS_HASH); - let bodies: HashMap> = - blocks.iter().map(body_by_hash).collect(); - let mut runner = BodyTestRunner::new(|| TestBodyDownloader::new(bodies.clone())); + let (stage_progress, previous_stage) = (1, 21); + + // Set up test runner + let mut runner = BodyTestRunner::default(); + let input = ExecInput { + previous_stage: Some((PREV_STAGE_ID, previous_stage)), + stage_progress: Some(stage_progress), + }; + runner.seed_execution(input).expect("failed to seed execution"); // Set the batch size to more than what the previous stage synced (40 vs 20) runner.set_batch_size(40); - // Insert required state - runner.insert_genesis().expect("Could not insert genesis block"); - runner - .insert_headers(blocks.iter().map(|block| &block.header)) - .expect("Could not insert headers"); - // Run the stage - let input = ExecInput { - previous_stage: Some((PREV_STAGE_ID, blocks.len() as BlockNumber)), - stage_progress: None, - }; let rx = runner.execute(input); // Check that we synced all blocks successfully, even though our `batch_size` allows us to @@ -343,23 +317,18 @@ mod tests { /// Same as [full_body_download] except we have made progress before #[tokio::test] async fn sync_from_previous_progress() { - // Generate blocks #1-20 - let blocks = random_block_range(1..21, GENESIS_HASH); - let bodies: HashMap> = - blocks.iter().map(body_by_hash).collect(); - let runner = BodyTestRunner::new(|| TestBodyDownloader::new(bodies.clone())); - - // Insert required state - runner.insert_genesis().expect("Could not insert genesis block"); - runner - .insert_headers(blocks.iter().map(|block| &block.header)) - .expect("Could not insert headers"); + let (stage_progress, previous_stage) = (1, 21); + + // Set up test runner + let mut runner = BodyTestRunner::default(); + let input = ExecInput { + previous_stage: Some((PREV_STAGE_ID, previous_stage)), + stage_progress: Some(stage_progress), + }; + runner.seed_execution(input).expect("failed to seed execution"); // Run the stage - let rx = runner.execute(ExecInput { - previous_stage: Some((PREV_STAGE_ID, blocks.len() as BlockNumber)), - stage_progress: None, - }); + let rx = runner.execute(input); // Check that we synced at least 10 blocks let first_run = rx.await.unwrap(); @@ -371,7 +340,7 @@ mod tests { // Execute again on top of the previous run let input = ExecInput { - previous_stage: Some((PREV_STAGE_ID, blocks.len() as BlockNumber)), + previous_stage: Some((PREV_STAGE_ID, previous_stage)), stage_progress: Some(first_run_progress), }; let rx = runner.execute(input); @@ -388,118 +357,48 @@ mod tests { /// Checks that the stage asks to unwind if pre-validation of the block fails. #[tokio::test] async fn pre_validation_failure() { - // Generate blocks #1-19 - let blocks = random_block_range(1..20, GENESIS_HASH); - let bodies: HashMap> = - blocks.iter().map(body_by_hash).collect(); - let mut runner = BodyTestRunner::new(|| TestBodyDownloader::new(bodies.clone())); + let (stage_progress, previous_stage) = (1, 20); - // Fail validation - runner.set_fail_validation(true); + // Set up test runner + let mut runner = BodyTestRunner::default(); + let input = ExecInput { + previous_stage: Some((PREV_STAGE_ID, previous_stage)), + stage_progress: Some(stage_progress), + }; + runner.seed_execution(input).expect("failed to seed execution"); - // Insert required state - runner.insert_genesis().expect("Could not insert genesis block"); - runner - .insert_headers(blocks.iter().map(|block| &block.header)) - .expect("Could not insert headers"); + // Fail validation + runner.consensus.set_fail_validation(true); // Run the stage - let rx = runner.execute(ExecInput { - previous_stage: Some((PREV_STAGE_ID, blocks.len() as BlockNumber)), - stage_progress: None, - }); + let rx = runner.execute(input); // Check that the error bubbles up assert_matches!( rx.await.unwrap(), - Err(StageError::Validation { block: 1, error: consensus::Error::BaseFeeMissing }) - ); - } - - /// Checks that the stage unwinds correctly with no data. - #[tokio::test] - async fn unwind_empty_db() { - let unwind_to = 10; - let runner = BodyTestRunner::new(TestBodyDownloader::default); - let input = UnwindInput { bad_block: None, stage_progress: 20, unwind_to }; - let rx = runner.unwind(input); - assert_matches!( - rx.await.unwrap(), - Ok(UnwindOutput { stage_progress }) if stage_progress == unwind_to + Err(StageError::Validation { error: consensus::Error::BaseFeeMissing, .. }) ); - assert!(runner.validate_unwind(input).is_ok(), "unwind validation"); - } - - /// Checks that the stage unwinds correctly with data. - #[tokio::test] - async fn unwind() { - // Generate blocks #1-20 - let blocks = random_block_range(1..21, GENESIS_HASH); - let bodies: HashMap> = - blocks.iter().map(body_by_hash).collect(); - let mut runner = BodyTestRunner::new(|| TestBodyDownloader::new(bodies.clone())); - - // Set the batch size to more than what the previous stage synced (40 vs 20) - runner.set_batch_size(40); - - // Insert required state - runner.insert_genesis().expect("Could not insert genesis block"); - runner - .insert_headers(blocks.iter().map(|block| &block.header)) - .expect("Could not insert headers"); - - // Run the stage - let execute_input = ExecInput { - previous_stage: Some((PREV_STAGE_ID, blocks.len() as BlockNumber)), - stage_progress: None, - }; - let rx = runner.execute(execute_input); - - // Check that we synced all blocks successfully, even though our `batch_size` allows us to - // sync more (if there were more headers) - let output = rx.await.unwrap(); - assert_matches!( - output, - Ok(ExecOutput { stage_progress: 20, reached_tip: true, done: true }) - ); - let output = output.unwrap(); - assert!(runner.validate_execution(execute_input, Some(output.clone())).is_ok(), "execution validation"); - - // Unwind all of it - let unwind_to = 1; - let unwind_input = UnwindInput { bad_block: None, stage_progress: output.stage_progress, unwind_to }; - let rx = runner.unwind(unwind_input); - assert_matches!( - rx.await.unwrap(), - Ok(UnwindOutput { stage_progress }) if stage_progress == 1 - ); - - assert!(runner.validate_unwind(unwind_input).is_ok(), "unwind validation"); + assert!(runner.validate_execution(input, None).is_ok(), "execution validation"); } /// Checks that the stage unwinds correctly, even if a transaction in a block is missing. #[tokio::test] async fn unwind_missing_tx() { - // Generate blocks #1-20 - let blocks = random_block_range(1..21, GENESIS_HASH); - let bodies: HashMap> = - blocks.iter().map(body_by_hash).collect(); - let mut runner = BodyTestRunner::new(|| TestBodyDownloader::new(bodies.clone())); + let (stage_progress, previous_stage) = (1, 21); + + // Set up test runner + let mut runner = BodyTestRunner::default(); + let input = ExecInput { + previous_stage: Some((PREV_STAGE_ID, previous_stage)), + stage_progress: Some(stage_progress), + }; + runner.seed_execution(input).expect("failed to seed execution"); // Set the batch size to more than what the previous stage synced (40 vs 20) runner.set_batch_size(40); - // Insert required state - runner.insert_genesis().expect("Could not insert genesis block"); - runner - .insert_headers(blocks.iter().map(|block| &block.header)) - .expect("Could not insert headers"); - // Run the stage - let rx = runner.execute(ExecInput { - previous_stage: Some((PREV_STAGE_ID, blocks.len() as BlockNumber)), - stage_progress: None, - }); + let rx = runner.execute(input); // Check that we synced all blocks successfully, even though our `batch_size` allows us to // sync more (if there were more headers) @@ -536,42 +435,42 @@ mod tests { /// try again? #[tokio::test] async fn downloader_timeout() { - // Generate a header - let header = random_block(1, Some(GENESIS_HASH)).header; - let runner = BodyTestRunner::new(|| { - TestBodyDownloader::new(HashMap::from([( - header.hash(), - Err(DownloadError::Timeout { header_hash: header.hash() }), - )])) - }); - - // Insert required state - runner.insert_genesis().expect("Could not insert genesis block"); - runner.insert_header(&header).expect("Could not insert header"); + let (stage_progress, previous_stage) = (1, 3); + + // Set up test runner + let mut runner = BodyTestRunner::default(); + let input = ExecInput { + previous_stage: Some((PREV_STAGE_ID, previous_stage)), + stage_progress: Some(stage_progress), // TODO: None? + }; + let blocks = runner.seed_execution(input).expect("failed to seed execution"); + + // overwrite responses + let header = blocks.last().unwrap(); + runner.set_responses(HashMap::from([( + header.hash(), + Err(DownloadError::Timeout { header_hash: header.hash() }), + )])); // Run the stage - let rx = runner.execute(ExecInput { - previous_stage: Some((PREV_STAGE_ID, 1)), - stage_progress: None, - }); + let rx = runner.execute(input); // Check that the error bubbles up assert_matches!(rx.await.unwrap(), Err(StageError::Internal(_))); + assert!(runner.validate_execution(input, None).is_ok(), "execution validation"); } mod test_utils { use crate::{ stages::bodies::BodyStage, - util::test_utils::{ + test_utils::{ ExecuteStageTestRunner, StageTestDB, StageTestRunner, UnwindStageTestRunner, TestRunnerError, }, ExecInput, UnwindInput, ExecOutput, }; use assert_matches::assert_matches; - use async_trait::async_trait; use reth_eth_wire::BlockBody; use reth_interfaces::{ - db, db::{ models::{BlockNumHash, StoredBlockBody}, tables, DbCursorRO, DbTx, DbTxMut, @@ -581,12 +480,12 @@ mod tests { downloader::{BodiesStream, BodyDownloader}, error::{BodiesClientError, DownloadError}, }, - test_utils::TestConsensus, + test_utils::{TestConsensus, generators::random_block_range}, }; use reth_primitives::{ BigEndianHash, BlockLocked, BlockNumber, Header, SealedHeader, H256, U256, }; - use std::{collections::HashMap, ops::Deref, time::Duration}; + use std::{collections::HashMap, ops::Deref, time::Duration, sync::Arc}; /// The block hash of the genesis block. pub(crate) const GENESIS_HASH: H256 = H256::zero(); @@ -605,43 +504,35 @@ mod tests { } /// A helper struct for running the [BodyStage]. - pub(crate) struct BodyTestRunner - where - F: Fn() -> TestBodyDownloader, - { - downloader_builder: F, + pub(crate) struct BodyTestRunner { + pub(crate) consensus: Arc, + responses: HashMap>, db: StageTestDB, batch_size: u64, - fail_validation: bool, } - impl BodyTestRunner - where - F: Fn() -> TestBodyDownloader, - { - /// Build a new test runner. - pub(crate) fn new(downloader_builder: F) -> Self { - BodyTestRunner { - downloader_builder, + impl Default for BodyTestRunner { + fn default() -> Self { + Self { + consensus: Arc::new(TestConsensus::default()), + responses: HashMap::default(), db: StageTestDB::default(), batch_size: 10, - fail_validation: false, } } + } + impl BodyTestRunner { pub(crate) fn set_batch_size(&mut self, batch_size: u64) { self.batch_size = batch_size; } - pub(crate) fn set_fail_validation(&mut self, fail_validation: bool) { - self.fail_validation = fail_validation; + pub(crate) fn set_responses(&mut self, responses: HashMap>) { + self.responses = responses; } } - impl StageTestRunner for BodyTestRunner - where - F: Fn() -> TestBodyDownloader, - { + impl StageTestRunner for BodyTestRunner { type S = BodyStage; fn db(&self) -> &StageTestDB { @@ -649,33 +540,29 @@ mod tests { } fn stage(&self) -> Self::S { - let mut consensus = TestConsensus::default(); - consensus.set_fail_validation(self.fail_validation); - BodyStage { - downloader: (self.downloader_builder)(), - consensus, + downloader: Arc::new(TestBodyDownloader::new(self.responses.clone())), + consensus: self.consensus.clone(), batch_size: self.batch_size, } } } - impl ExecuteStageTestRunner for BodyTestRunner - where - F: Fn() -> TestBodyDownloader, - { - type Seed = (); + #[async_trait::async_trait] + impl ExecuteStageTestRunner for BodyTestRunner { + type Seed = Vec; fn seed_execution( &mut self, input: ExecInput, - ) -> Result<(), TestRunnerError> { + ) -> Result { + let start = input.stage_progress.unwrap_or_default(); + let end = input.previous_stage.as_ref().map(|(_, num)| *num).unwrap_or_default(); + let blocks = random_block_range(start..end, GENESIS_HASH); self.insert_genesis()?; - // TODO: - // self - // .insert_headers(blocks.iter().map(|block| &block.header)) - // .expect("Could not insert headers"); - Ok(()) + self.insert_headers(blocks.iter().map(|block| &block.header))?; + self.set_responses(blocks.iter().map(body_by_hash).collect()); + Ok(blocks) } fn validate_execution( @@ -683,31 +570,21 @@ mod tests { input: ExecInput, output: Option, ) -> Result<(), TestRunnerError> { - if let Some(output) = output { - self.validate_db_blocks(output.stage_progress)?; - } - Ok(()) + let highest_block = match output.as_ref() { + Some(output) => output.stage_progress, + None => input.stage_progress.unwrap_or_default(), + }; + self.validate_db_blocks(highest_block) } } - impl UnwindStageTestRunner for BodyTestRunner - where - F: Fn() -> TestBodyDownloader, - { - fn seed_unwind( - &mut self, - input: UnwindInput, - highest_entry: u64, - ) -> Result<(), TestRunnerError> { - unimplemented!() - } - + impl UnwindStageTestRunner for BodyTestRunner { fn validate_unwind( &self, input: UnwindInput, ) -> Result<(), TestRunnerError> { self.db.check_no_entry_above::(input.unwind_to, |key| key.number())?; - if let Some(last_body) =self.last_body(){ + if let Some(last_body) = self.last_body() { let last_tx_id = last_body.base_tx_id + last_body.tx_amount; self.db.check_no_entry_above::(last_tx_id, |key| key)?; } @@ -715,15 +592,12 @@ mod tests { } } - impl BodyTestRunner - where - F: Fn() -> TestBodyDownloader, - { + impl BodyTestRunner { /// Insert the genesis block into the appropriate tables /// /// The genesis block always has no transactions and no ommers, and it always has the /// same hash. - pub(crate) fn insert_genesis(&self) -> Result<(), db::Error> { + pub(crate) fn insert_genesis(&self) -> Result<(), TestRunnerError> { self.insert_header(&SealedHeader::new(Header::default(), GENESIS_HASH))?; self.db.commit(|tx| { tx.put::( @@ -736,13 +610,14 @@ mod tests { } /// Insert header into tables - pub(crate) fn insert_header(&self, header: &SealedHeader) -> Result<(), db::Error> { - self.insert_headers(std::iter::once(header)) + pub(crate) fn insert_header(&self, header: &SealedHeader) -> Result<(), TestRunnerError> { + self.insert_headers(std::iter::once(header))?; + Ok(()) } /// Insert headers into tables /// TODO: move to common inserter - pub(crate) fn insert_headers<'a, I>(&self, headers: I) -> Result<(), db::Error> + pub(crate) fn insert_headers<'a, I>(&self, headers: I) -> Result<(), TestRunnerError> where I: Iterator, { @@ -778,7 +653,7 @@ mod tests { pub(crate) fn validate_db_blocks( &self, highest_block: BlockNumber, - ) -> Result<(), db::Error> { + ) -> Result<(), TestRunnerError> { self.db.query(|tx| { let mut block_body_cursor = tx.cursor::()?; let mut transaction_cursor = tx.cursor::()?; @@ -806,7 +681,8 @@ mod tests { } Ok(()) - }) + })?; + Ok(()) } } @@ -815,7 +691,7 @@ mod tests { #[derive(Debug)] pub(crate) struct NoopClient; - #[async_trait] + #[async_trait::async_trait] impl BodiesClient for NoopClient { async fn get_block_body(&self, _: H256) -> Result { panic!("Noop client should not be called") @@ -824,7 +700,7 @@ mod tests { // TODO(onbjerg): Move /// A [BodyDownloader] that is backed by an internal [HashMap] for testing. - #[derive(Debug, Default)] + #[derive(Debug, Default, Clone)] pub(crate) struct TestBodyDownloader { responses: HashMap>, } @@ -854,14 +730,11 @@ mod tests { { Box::pin(futures_util::stream::iter(hashes.into_iter().map( |(block_number, hash)| { - Ok(( - *block_number, - *hash, - self.responses - .get(hash) - .expect("Stage tried downloading a block we do not have.") - .clone()?, - )) + let result = self.responses + .get(hash) + .expect("Stage tried downloading a block we do not have.") + .clone()?; + Ok((*block_number, *hash, result)) }, ))) } diff --git a/crates/stages/src/stages/headers.rs b/crates/stages/src/stages/headers.rs index 0ea0c86dea5..49039a4c0cf 100644 --- a/crates/stages/src/stages/headers.rs +++ b/crates/stages/src/stages/headers.rs @@ -190,7 +190,7 @@ impl HeaderStage { #[cfg(test)] mod tests { use super::*; - use crate::util::test_utils::{ + use crate::test_utils::{ stage_test_suite, ExecuteStageTestRunner, UnwindStageTestRunner, PREV_STAGE_ID, }; use assert_matches::assert_matches; @@ -198,8 +198,6 @@ mod tests { stage_test_suite!(HeadersTestRunner); - // TODO: test consensus propagation error - /// Check that the execution errors on empty database or /// prev progress missing from the database. #[tokio::test] @@ -218,6 +216,20 @@ mod tests { assert!(runner.validate_execution(input, result.ok()).is_ok(), "validation failed"); } + /// Check that validation error is propagated during the execution. + #[tokio::test] + async fn execute_validation_error() { + let mut runner = HeadersTestRunner::default(); + runner.consensus.set_fail_validation(true); + let input = ExecInput::default(); + let seed = runner.seed_execution(input).expect("failed to seed execution"); + let rx = runner.execute(input); + runner.after_execution(seed).await.expect("failed to run after execution hook"); + let result = rx.await.unwrap(); + assert_matches!(result, Err(StageError::Validation { .. })); + assert!(runner.validate_execution(input, result.ok()).is_ok(), "validation failed"); + } + /// Execute the stage with linear downloader #[tokio::test] async fn execute_with_linear_downloader() { @@ -254,7 +266,7 @@ mod tests { mod test_runner { use crate::{ stages::headers::HeaderStage, - util::test_utils::{ + test_utils::{ ExecuteStageTestRunner, StageTestDB, StageTestRunner, TestRunnerError, UnwindStageTestRunner, }, @@ -262,14 +274,14 @@ mod tests { }; use reth_headers_downloaders::linear::{LinearDownloadBuilder, LinearDownloader}; use reth_interfaces::{ - db::{self, models::blocks::BlockNumHash, tables, DbTx}, + db::{models::blocks::BlockNumHash, tables, DbTx}, p2p::headers::downloader::HeaderDownloader, test_utils::{ generators::{random_header, random_header_range}, TestConsensus, TestHeaderDownloader, TestHeadersClient, }, }; - use reth_primitives::{rpc::BigEndianHash, SealedHeader, H256, U256}; + use reth_primitives::{rpc::BigEndianHash, BlockNumber, SealedHeader, H256, U256}; use std::{ops::Deref, sync::Arc}; pub(crate) struct HeadersTestRunner { @@ -282,10 +294,11 @@ mod tests { impl Default for HeadersTestRunner { fn default() -> Self { let client = Arc::new(TestHeadersClient::default()); + let consensus = Arc::new(TestConsensus::default()); Self { client: client.clone(), - consensus: Arc::new(TestConsensus::default()), - downloader: Arc::new(TestHeaderDownloader::new(client)), + consensus: consensus.clone(), + downloader: Arc::new(TestHeaderDownloader::new(client, consensus)), db: StageTestDB::default(), } } @@ -345,45 +358,58 @@ mod tests { Ok(()) } + /// Validate stored headers fn validate_execution( &self, - _input: ExecInput, - _output: Option, + input: ExecInput, + output: Option, ) -> Result<(), TestRunnerError> { - // TODO: refine - // if let Some(ref headers) = self.context { - // // skip head and validate each - // headers.iter().skip(1).try_for_each(|h| self.validate_db_header(&h))?; - // } + let initial_stage_progress = input.stage_progress.unwrap_or_default(); + match output { + Some(output) if output.stage_progress > initial_stage_progress => { + self.db.query(|tx| { + for block_num in (initial_stage_progress..output.stage_progress).rev() { + // look up the header hash + let hash = tx + .get::(block_num)? + .expect("no header hash"); + let key: BlockNumHash = (block_num, hash).into(); + + // validate the header number + assert_eq!(tx.get::(hash)?, Some(block_num)); + + // validate the header + let header = tx.get::(key)?; + assert!(header.is_some()); + let header = header.unwrap().seal(); + assert_eq!(header.hash(), hash); + + // validate td consistency in the database + if header.number > initial_stage_progress { + let parent_td = tx.get::( + (header.number - 1, header.parent_hash).into(), + )?; + let td = tx.get::(key)?.unwrap(); + assert_eq!( + parent_td.map( + |td| U256::from_big_endian(&td) + header.difficulty + ), + Some(U256::from_big_endian(&td)) + ); + } + } + Ok(()) + })?; + } + _ => self.check_no_header_entry_above(initial_stage_progress)?, + }; Ok(()) } } impl UnwindStageTestRunner for HeadersTestRunner { - fn seed_unwind( - &mut self, - input: UnwindInput, - highest_entry: u64, - ) -> Result<(), TestRunnerError> { - let lowest_entry = input.unwind_to.saturating_sub(100); - let headers = random_header_range(lowest_entry..highest_entry, H256::zero()); - self.insert_headers(headers.iter())?; - Ok(()) - } - fn validate_unwind(&self, input: UnwindInput) -> Result<(), TestRunnerError> { - let unwind_to = input.unwind_to; - self.db.check_no_entry_above_by_value::( - unwind_to, - |val| val, - )?; - self.db - .check_no_entry_above::(unwind_to, |key| key)?; - self.db - .check_no_entry_above::(unwind_to, |key| key.number())?; - self.db - .check_no_entry_above::(unwind_to, |key| key.number())?; - Ok(()) + self.check_no_header_entry_above(input.unwind_to) } } @@ -400,12 +426,15 @@ mod tests { impl HeadersTestRunner { /// Insert header into tables - pub(crate) fn insert_header(&self, header: &SealedHeader) -> Result<(), db::Error> { + pub(crate) fn insert_header( + &self, + header: &SealedHeader, + ) -> Result<(), TestRunnerError> { self.insert_headers(std::iter::once(header)) } /// Insert headers into tables - pub(crate) fn insert_headers<'a, I>(&self, headers: I) -> Result<(), db::Error> + pub(crate) fn insert_headers<'a, I>(&self, headers: I) -> Result<(), TestRunnerError> where I: Iterator, { @@ -430,36 +459,16 @@ mod tests { Ok(()) } - /// Validate stored header against provided - pub(crate) fn validate_db_header( + pub(crate) fn check_no_header_entry_above( &self, - header: &SealedHeader, - ) -> Result<(), db::Error> { - self.db.query(|tx| { - let key: BlockNumHash = (header.number, header.hash()).into(); - - let db_number = tx.get::(header.hash())?; - assert_eq!(db_number, Some(header.number)); - - let db_header = tx.get::(key)?; - assert_eq!(db_header, Some(header.clone().unseal())); - - let db_canonical_header = tx.get::(header.number)?; - assert_eq!(db_canonical_header, Some(header.hash())); - - if header.number != 0 { - let parent_key: BlockNumHash = - (header.number - 1, header.parent_hash).into(); - let parent_td = tx.get::(parent_key)?; - let td = U256::from_big_endian(&tx.get::(key)?.unwrap()); - assert_eq!( - parent_td.map(|td| U256::from_big_endian(&td) + header.difficulty), - Some(td) - ); - } - - Ok(()) - }) + block: BlockNumber, + ) -> Result<(), TestRunnerError> { + self.db + .check_no_entry_above_by_value::(block, |val| val)?; + self.db.check_no_entry_above::(block, |key| key)?; + self.db.check_no_entry_above::(block, |key| key.number())?; + self.db.check_no_entry_above::(block, |key| key.number())?; + Ok(()) } } } diff --git a/crates/stages/src/stages/tx_index.rs b/crates/stages/src/stages/tx_index.rs index fd4ac04f2e4..aec36100154 100644 --- a/crates/stages/src/stages/tx_index.rs +++ b/crates/stages/src/stages/tx_index.rs @@ -87,7 +87,7 @@ impl Stage for TxIndex { #[cfg(test)] mod tests { use super::*; - use crate::util::test_utils::{ + use crate::test_utils::{ stage_test_suite, ExecuteStageTestRunner, StageTestDB, StageTestRunner, TestRunnerError, UnwindStageTestRunner, }; @@ -96,7 +96,7 @@ mod tests { db::models::{BlockNumHash, StoredBlockBody}, test_utils::generators::random_header_range, }; - use reth_primitives::{SealedHeader, H256}; + use reth_primitives::H256; stage_test_suite!(TxIndexTestRunner); @@ -155,7 +155,7 @@ mod tests { fn validate_execution( &self, input: ExecInput, - output: Option, + _output: Option, ) -> Result<(), TestRunnerError> { self.db.query(|tx| { let (start, end) = ( @@ -187,21 +187,6 @@ mod tests { } impl UnwindStageTestRunner for TxIndexTestRunner { - fn seed_unwind( - &mut self, - input: UnwindInput, - highest_entry: u64, - ) -> Result<(), TestRunnerError> { - let headers = random_header_range(input.unwind_to..highest_entry, H256::zero()); - self.db.transform_append::(&headers, |prev, h| { - ( - BlockNumHash((h.number, h.hash())), - prev.unwrap_or_default() + (rand::random::() as u64), - ) - })?; - Ok(()) - } - fn validate_unwind(&self, input: UnwindInput) -> Result<(), TestRunnerError> { self.db.check_no_entry_above::(input.unwind_to, |h| { h.number() diff --git a/crates/stages/src/test_utils/macros.rs b/crates/stages/src/test_utils/macros.rs new file mode 100644 index 00000000000..bc2c8410b09 --- /dev/null +++ b/crates/stages/src/test_utils/macros.rs @@ -0,0 +1,106 @@ +// TODO: add comments +macro_rules! stage_test_suite { + ($runner:ident) => { + /// Check that the execution is short-circuited if the database is empty. + #[tokio::test] + async fn execute_empty_db() { + let runner = $runner::default(); + let input = crate::stage::ExecInput::default(); + let result = runner.execute(input).await.unwrap(); + assert_matches!( + result, + Err(crate::error::StageError::DatabaseIntegrity(_)) + ); + assert!(runner.validate_execution(input, result.ok()).is_ok(), "execution validation"); + } + + /// Check that the execution is short-circuited if the target was already reached. + #[tokio::test] + async fn execute_already_reached_target() { + let stage_progress = 1000; + let mut runner = $runner::default(); + let input = crate::stage::ExecInput { + previous_stage: Some((crate::test_utils::PREV_STAGE_ID, stage_progress)), + stage_progress: Some(stage_progress), + }; + let seed = runner.seed_execution(input).expect("failed to seed"); + let rx = runner.execute(input); + runner.after_execution(seed).await.expect("failed to run after execution hook"); + let result = rx.await.unwrap(); + assert_matches!( + result, + Ok(ExecOutput { done, reached_tip, stage_progress }) + if done && reached_tip && stage_progress == stage_progress + ); + assert!(runner.validate_execution(input, result.ok()).is_ok(), "execution validation"); + } + + #[tokio::test] + async fn execute() { + let (previous_stage, stage_progress) = (1000, 100); + let mut runner = $runner::default(); + let input = crate::stage::ExecInput { + previous_stage: Some((crate::test_utils::PREV_STAGE_ID, previous_stage)), + stage_progress: Some(stage_progress), + }; + let seed = runner.seed_execution(input).expect("failed to seed"); + let rx = runner.execute(input); + runner.after_execution(seed).await.expect("failed to run after execution hook"); + let result = rx.await.unwrap(); + println!("RESULT >>> {:?}", result.unwrap_err().to_string()); + // assert_matches!( + // result, + // Ok(ExecOutput { done, reached_tip, stage_progress }) + // if done && reached_tip && stage_progress == previous_stage + // ); + // assert!(runner.validate_execution(input, result.ok()).is_ok(), "execution validation"); + } + + #[tokio::test] + // Check that unwind does not panic on empty database. + async fn unwind_empty_db() { + let runner = $runner::default(); + let input = crate::stage::UnwindInput::default(); + let rx = runner.unwind(input); + assert_matches!( + rx.await.unwrap(), + Ok(UnwindOutput { stage_progress }) if stage_progress == input.unwind_to + ); + assert!(runner.validate_unwind(input).is_ok(), "unwind validation"); + } + + #[tokio::test] + async fn unwind() { + let (previous_stage, stage_progress) = (1000, 100); + let mut runner = $runner::default(); + + // Run execute + let execute_input = crate::stage::ExecInput { + previous_stage: Some((crate::test_utils::PREV_STAGE_ID, previous_stage)), + stage_progress: Some(stage_progress), + }; + let seed = runner.seed_execution(execute_input).expect("failed to seed"); + let rx = runner.execute(execute_input); + runner.after_execution(seed).await.expect("failed to run after execution hook"); + let result = rx.await.unwrap(); + assert_matches!( + result, + Ok(ExecOutput { done, reached_tip, stage_progress }) + if done && reached_tip && stage_progress == previous_stage + ); + assert!(runner.validate_execution(execute_input, result.ok()).is_ok(), "execution validation"); + + let unwind_input = crate::stage::UnwindInput { + unwind_to: stage_progress, stage_progress, bad_block: None, + }; + let rx = runner.unwind(unwind_input); + assert_matches!( + rx.await.unwrap(), + Ok(UnwindOutput { stage_progress }) if stage_progress == unwind_input.unwind_to + ); + assert!(runner.validate_unwind(unwind_input).is_ok(), "unwind validation"); + } + }; +} + +pub(crate) use stage_test_suite; diff --git a/crates/stages/src/test_utils/mod.rs b/crates/stages/src/test_utils/mod.rs new file mode 100644 index 00000000000..5fae3794f5c --- /dev/null +++ b/crates/stages/src/test_utils/mod.rs @@ -0,0 +1,14 @@ +use crate::StageId; + +mod macros; +mod runner; +mod stage_db; + +pub(crate) use macros::*; +pub(crate) use runner::{ + ExecuteStageTestRunner, StageTestRunner, TestRunnerError, UnwindStageTestRunner, +}; +pub(crate) use stage_db::StageTestDB; + +/// The previous test stage id mock used for testing +pub(crate) const PREV_STAGE_ID: StageId = StageId("PrevStage"); diff --git a/crates/stages/src/test_utils/runner.rs b/crates/stages/src/test_utils/runner.rs new file mode 100644 index 00000000000..c81574dbd5f --- /dev/null +++ b/crates/stages/src/test_utils/runner.rs @@ -0,0 +1,81 @@ +use reth_db::{kv::Env, mdbx::WriteMap}; +use reth_interfaces::db::{self, DBContainer}; +use std::borrow::Borrow; +use tokio::sync::oneshot; + +use super::StageTestDB; +use crate::{ExecInput, ExecOutput, Stage, StageError, UnwindInput, UnwindOutput}; + +#[derive(thiserror::Error, Debug)] +pub(crate) enum TestRunnerError { + #[error("Database error occured.")] + Database(#[from] db::Error), + #[error("Internal runner error occured.")] + Internal(#[from] Box), +} + +/// A generic test runner for stages. +#[async_trait::async_trait] +pub(crate) trait StageTestRunner { + type S: Stage> + 'static; + + /// Return a reference to the database. + fn db(&self) -> &StageTestDB; + + /// Return an instance of a Stage. + fn stage(&self) -> Self::S; +} + +#[async_trait::async_trait] +pub(crate) trait ExecuteStageTestRunner: StageTestRunner { + type Seed: Send + Sync; + + /// Seed database for stage execution + fn seed_execution(&mut self, input: ExecInput) -> Result; + + /// Validate stage execution + fn validate_execution( + &self, + input: ExecInput, + output: Option, + ) -> Result<(), TestRunnerError>; + + /// Run [Stage::execute] and return a receiver for the result. + fn execute(&self, input: ExecInput) -> oneshot::Receiver> { + let (tx, rx) = oneshot::channel(); + let (db, mut stage) = (self.db().inner(), self.stage()); + tokio::spawn(async move { + let mut db = DBContainer::new(db.borrow()).expect("failed to create db container"); + let result = stage.execute(&mut db, input).await; + db.commit().expect("failed to commit"); + tx.send(result).expect("failed to send message") + }); + rx + } + + /// Run a hook after [Stage::execute]. Required for Headers & Bodies stages. + async fn after_execution(&self, _seed: Self::Seed) -> Result<(), TestRunnerError> { + Ok(()) + } +} + +pub(crate) trait UnwindStageTestRunner: StageTestRunner { + /// Validate the unwind + fn validate_unwind(&self, input: UnwindInput) -> Result<(), TestRunnerError>; + + /// Run [Stage::unwind] and return a receiver for the result. + fn unwind( + &self, + input: UnwindInput, + ) -> oneshot::Receiver>> { + let (tx, rx) = oneshot::channel(); + let (db, mut stage) = (self.db().inner(), self.stage()); + tokio::spawn(async move { + let mut db = DBContainer::new(db.borrow()).expect("failed to create db container"); + let result = stage.unwind(&mut db, input).await; + db.commit().expect("failed to commit"); + tx.send(result).expect("failed to send result"); + }); + rx + } +} diff --git a/crates/stages/src/test_utils/stage_db.rs b/crates/stages/src/test_utils/stage_db.rs new file mode 100644 index 00000000000..f14b1cb4f2f --- /dev/null +++ b/crates/stages/src/test_utils/stage_db.rs @@ -0,0 +1,149 @@ +use reth_db::{ + kv::{test_utils::create_test_db, tx::Tx, Env, EnvKind}, + mdbx::{WriteMap, RW}, +}; +use reth_interfaces::db::{self, DBContainer, DbCursorRO, DbCursorRW, DbTx, DbTxMut, Table}; +use reth_primitives::BlockNumber; +use std::{borrow::Borrow, sync::Arc}; + +/// The [StageTestDB] is used as an internal +/// database for testing stage implementation. +/// +/// ```rust +/// let db = StageTestDB::default(); +/// stage.execute(&mut db.container(), input); +/// ``` +pub(crate) struct StageTestDB { + db: Arc>, +} + +impl Default for StageTestDB { + /// Create a new instance of [StageTestDB] + fn default() -> Self { + Self { db: create_test_db::(EnvKind::RW) } + } +} + +impl StageTestDB { + /// Return a database wrapped in [DBContainer]. + fn container(&self) -> DBContainer<'_, Env> { + DBContainer::new(self.db.borrow()).expect("failed to create db container") + } + + /// Get a pointer to an internal database. + pub(crate) fn inner(&self) -> Arc> { + self.db.clone() + } + + /// Invoke a callback with transaction committing it afterwards + pub(crate) fn commit(&self, f: F) -> Result<(), db::Error> + where + F: FnOnce(&mut Tx<'_, RW, WriteMap>) -> Result<(), db::Error>, + { + let mut db = self.container(); + let tx = db.get_mut(); + f(tx)?; + db.commit()?; + Ok(()) + } + + /// Invoke a callback with a read transaction + pub(crate) fn query(&self, f: F) -> Result + where + F: FnOnce(&Tx<'_, RW, WriteMap>) -> Result, + { + f(self.container().get()) + } + + /// Map a collection of values and store them in the database. + /// This function commits the transaction before exiting. + /// + /// ```rust + /// let db = StageTestDB::default(); + /// db.map_put::(&items, |item| item)?; + /// ``` + pub(crate) fn map_put(&self, values: &[S], mut map: F) -> Result<(), db::Error> + where + T: Table, + S: Clone, + F: FnMut(&S) -> (T::Key, T::Value), + { + self.commit(|tx| { + values.iter().try_for_each(|src| { + let (k, v) = map(src); + tx.put::(k, v) + }) + }) + } + + /// Transform a collection of values using a callback and store + /// them in the database. The callback additionally accepts the + /// optional last element that was stored. + /// This function commits the transaction before exiting. + /// + /// ```rust + /// let db = StageTestDB::default(); + /// db.transform_append::(&items, |prev, item| prev.unwrap_or_default() + item)?; + /// ``` + pub(crate) fn transform_append( + &self, + values: &[S], + mut transform: F, + ) -> Result<(), db::Error> + where + T: Table, + ::Value: Clone, + S: Clone, + F: FnMut(&Option<::Value>, &S) -> (T::Key, T::Value), + { + self.commit(|tx| { + let mut cursor = tx.cursor_mut::()?; + let mut last = cursor.last()?.map(|(_, v)| v); + values.iter().try_for_each(|src| { + let (k, v) = transform(&last, src); + last = Some(v.clone()); + cursor.append(k, v) + }) + }) + } + + /// Check that there is no table entry above a given + /// block by [Table::Key] + pub(crate) fn check_no_entry_above( + &self, + block: BlockNumber, + mut selector: F, + ) -> Result<(), db::Error> + where + T: Table, + F: FnMut(T::Key) -> BlockNumber, + { + self.query(|tx| { + let mut cursor = tx.cursor::()?; + if let Some((key, _)) = cursor.last()? { + assert!(selector(key) <= block); + } + Ok(()) + }) + } + + /// Check that there is no table entry above a given + /// block by [Table::Value] + pub(crate) fn check_no_entry_above_by_value( + &self, + block: BlockNumber, + mut selector: F, + ) -> Result<(), db::Error> + where + T: Table, + F: FnMut(T::Value) -> BlockNumber, + { + self.query(|tx| { + let mut cursor = tx.cursor::()?; + if let Some((_, value)) = cursor.last()? { + assert!(selector(value) <= block); + } + Ok(()) + }) + } +} diff --git a/crates/stages/src/util.rs b/crates/stages/src/util.rs index 6c5497385d3..62ec6a93226 100644 --- a/crates/stages/src/util.rs +++ b/crates/stages/src/util.rs @@ -135,332 +135,3 @@ pub(crate) mod unwind { Ok(()) } } - -#[cfg(test)] -pub(crate) mod test_utils { - use reth_db::{ - kv::{test_utils::create_test_db, tx::Tx, Env, EnvKind}, - mdbx::{WriteMap, RW}, - }; - use reth_interfaces::db::{self, DBContainer, DbCursorRO, DbCursorRW, DbTx, DbTxMut, Table}; - use reth_primitives::BlockNumber; - use std::{borrow::Borrow, sync::Arc}; - use tokio::sync::oneshot; - - use crate::{ExecInput, ExecOutput, Stage, StageError, StageId, UnwindInput, UnwindOutput}; - - /// The previous test stage id mock used for testing - pub(crate) const PREV_STAGE_ID: StageId = StageId("PrevStage"); - - /// The [StageTestDB] is used as an internal - /// database for testing stage implementation. - /// - /// ```rust - /// let db = StageTestDB::default(); - /// stage.execute(&mut db.container(), input); - /// ``` - pub(crate) struct StageTestDB { - db: Arc>, - } - - impl Default for StageTestDB { - /// Create a new instance of [StageTestDB] - fn default() -> Self { - Self { db: create_test_db::(EnvKind::RW) } - } - } - - impl StageTestDB { - /// Return a database wrapped in [DBContainer]. - fn container(&self) -> DBContainer<'_, Env> { - DBContainer::new(self.db.borrow()).expect("failed to create db container") - } - - /// Get a pointer to an internal database. - pub(crate) fn inner(&self) -> Arc> { - self.db.clone() - } - - /// Invoke a callback with transaction committing it afterwards - pub(crate) fn commit(&self, f: F) -> Result<(), db::Error> - where - F: FnOnce(&mut Tx<'_, RW, WriteMap>) -> Result<(), db::Error>, - { - let mut db = self.container(); - let tx = db.get_mut(); - f(tx)?; - db.commit()?; - Ok(()) - } - - /// Invoke a callback with a read transaction - pub(crate) fn query(&self, f: F) -> Result - where - F: FnOnce(&Tx<'_, RW, WriteMap>) -> Result, - { - f(self.container().get()) - } - - /// Map a collection of values and store them in the database. - /// This function commits the transaction before exiting. - /// - /// ```rust - /// let db = StageTestDB::default(); - /// db.map_put::(&items, |item| item)?; - /// ``` - pub(crate) fn map_put(&self, values: &[S], mut map: F) -> Result<(), db::Error> - where - T: Table, - S: Clone, - F: FnMut(&S) -> (T::Key, T::Value), - { - self.commit(|tx| { - values.iter().try_for_each(|src| { - let (k, v) = map(src); - tx.put::(k, v) - }) - }) - } - - /// Transform a collection of values using a callback and store - /// them in the database. The callback additionally accepts the - /// optional last element that was stored. - /// This function commits the transaction before exiting. - /// - /// ```rust - /// let db = StageTestDB::default(); - /// db.transform_append::(&items, |prev, item| prev.unwrap_or_default() + item)?; - /// ``` - pub(crate) fn transform_append( - &self, - values: &[S], - mut transform: F, - ) -> Result<(), db::Error> - where - T: Table, - ::Value: Clone, - S: Clone, - F: FnMut(&Option<::Value>, &S) -> (T::Key, T::Value), - { - self.commit(|tx| { - let mut cursor = tx.cursor_mut::()?; - let mut last = cursor.last()?.map(|(_, v)| v); - values.iter().try_for_each(|src| { - let (k, v) = transform(&last, src); - last = Some(v.clone()); - cursor.append(k, v) - }) - }) - } - - /// Check that there is no table entry above a given - /// block by [Table::Key] - pub(crate) fn check_no_entry_above( - &self, - block: BlockNumber, - mut selector: F, - ) -> Result<(), db::Error> - where - T: Table, - F: FnMut(T::Key) -> BlockNumber, - { - self.query(|tx| { - let mut cursor = tx.cursor::()?; - if let Some((key, _)) = cursor.last()? { - assert!(selector(key) <= block); - } - Ok(()) - }) - } - - /// Check that there is no table entry above a given - /// block by [Table::Value] - pub(crate) fn check_no_entry_above_by_value( - &self, - block: BlockNumber, - mut selector: F, - ) -> Result<(), db::Error> - where - T: Table, - F: FnMut(T::Value) -> BlockNumber, - { - self.query(|tx| { - let mut cursor = tx.cursor::()?; - if let Some((_, value)) = cursor.last()? { - assert!(selector(value) <= block); - } - Ok(()) - }) - } - } - - #[derive(thiserror::Error, Debug)] - pub(crate) enum TestRunnerError { - #[error("Database error occured.")] - Database(#[from] db::Error), - #[error("Internal runner error occured.")] - Internal(#[from] Box), - } - - /// A generic test runner for stages. - #[async_trait::async_trait] - pub(crate) trait StageTestRunner { - type S: Stage> + 'static; - - /// Return a reference to the database. - fn db(&self) -> &StageTestDB; - - /// Return an instance of a Stage. - fn stage(&self) -> Self::S; - } - - #[async_trait::async_trait] - pub(crate) trait ExecuteStageTestRunner: StageTestRunner { - type Seed: Send + Sync; - - /// Seed database for stage execution - fn seed_execution(&mut self, input: ExecInput) -> Result; - - /// Validate stage execution - fn validate_execution( - &self, - input: ExecInput, - output: Option, - ) -> Result<(), TestRunnerError>; - - /// Run [Stage::execute] and return a receiver for the result. - fn execute(&self, input: ExecInput) -> oneshot::Receiver> { - let (tx, rx) = oneshot::channel(); - let (db, mut stage) = (self.db().inner(), self.stage()); - tokio::spawn(async move { - let mut db = DBContainer::new(db.borrow()).expect("failed to create db container"); - let result = stage.execute(&mut db, input).await; - db.commit().expect("failed to commit"); - tx.send(result).expect("failed to send message") - }); - rx - } - - /// Run a hook after [Stage::execute]. Required for Headers & Bodies stages. - async fn after_execution(&self, seed: Self::Seed) -> Result<(), TestRunnerError> { - Ok(()) - } - } - - pub(crate) trait UnwindStageTestRunner: StageTestRunner { - /// Seed database for stage unwind - fn seed_unwind( - &mut self, - input: UnwindInput, - highest_entry: u64, - ) -> Result<(), TestRunnerError>; - - /// Validate the unwind - fn validate_unwind(&self, input: UnwindInput) -> Result<(), TestRunnerError>; - - /// Run [Stage::unwind] and return a receiver for the result. - fn unwind( - &self, - input: UnwindInput, - ) -> oneshot::Receiver>> - { - let (tx, rx) = oneshot::channel(); - let (db, mut stage) = (self.db().inner(), self.stage()); - tokio::spawn(async move { - let mut db = DBContainer::new(db.borrow()).expect("failed to create db container"); - let result = stage.unwind(&mut db, input).await; - db.commit().expect("failed to commit"); - tx.send(result).expect("failed to send result"); - }); - rx - } - } - - macro_rules! stage_test_suite { - ($runner:ident) => { - /// Check that the execution is short-circuited if the database is empty. - #[tokio::test] - async fn execute_empty_db() { - let runner = $runner::default(); - let input = crate::stage::ExecInput::default(); - let result = runner.execute(input).await.unwrap(); - assert_matches!( - result, - Err(crate::error::StageError::DatabaseIntegrity(_)) - ); - assert!(runner.validate_execution(input, result.ok()).is_ok(), "execution validation"); - } - - #[tokio::test] - async fn execute_already_reached_target() { - let stage_progress = 1000; - let mut runner = $runner::default(); - let input = crate::stage::ExecInput { - previous_stage: Some((crate::util::test_utils::PREV_STAGE_ID, stage_progress)), - stage_progress: Some(stage_progress), - }; - let seed = runner.seed_execution(input).expect("failed to seed"); - let rx = runner.execute(input); - runner.after_execution(seed).await.expect("failed to run after execution hook"); - let result = rx.await.unwrap(); - assert_matches!( - result, - Ok(ExecOutput { done, reached_tip, stage_progress }) - if done && reached_tip && stage_progress == stage_progress - ); - assert!(runner.validate_execution(input, result.ok()).is_ok(), "execution validation"); - } - - #[tokio::test] - async fn execute() { - let (previous_stage, stage_progress) = (1000, 100); - let mut runner = $runner::default(); - let input = crate::stage::ExecInput { - previous_stage: Some((crate::util::test_utils::PREV_STAGE_ID, previous_stage)), - stage_progress: Some(stage_progress), - }; - let seed = runner.seed_execution(input).expect("failed to seed"); - let rx = runner.execute(input); - runner.after_execution(seed).await.expect("failed to run after execution hook"); - let result = rx.await.unwrap(); - assert_matches!( - result, - Ok(ExecOutput { done, reached_tip, stage_progress }) - if done && reached_tip && stage_progress == previous_stage - ); - assert!(runner.validate_execution(input, result.ok()).is_ok(), "execution validation"); - } - - #[tokio::test] - // Check that unwind does not panic on empty database. - async fn unwind_empty_db() { - let runner = $runner::default(); - let input = crate::stage::UnwindInput::default(); - let rx = runner.unwind(input); - assert_matches!( - rx.await.unwrap(), - Ok(UnwindOutput { stage_progress }) if stage_progress == input.unwind_to - ); - assert!(runner.validate_unwind(input).is_ok(), "unwind validation"); - } - - #[tokio::test] - async fn unwind() { - let (unwind_to, highest_entry) = (100, 200); - let mut runner = $runner::default(); - let input = crate::stage::UnwindInput { - unwind_to, stage_progress: unwind_to, bad_block: None, - }; - runner.seed_unwind(input, highest_entry).expect("failed to seed"); - let rx = runner.unwind(input); - assert_matches!( - rx.await.unwrap(), - Ok(UnwindOutput { stage_progress }) if stage_progress == input.unwind_to - ); - assert!(runner.validate_unwind(input).is_ok(), "unwind validation"); - } - }; - } - - pub(crate) use stage_test_suite; -} From cb0ffc175f18c14b06f75cd8a7cf963f5eb517e2 Mon Sep 17 00:00:00 2001 From: Roman Krasiuk Date: Thu, 17 Nov 2022 15:59:07 +0200 Subject: [PATCH 06/17] add comments to suite tests --- crates/stages/src/test_utils/macros.rs | 61 +++++++++++++++++++++----- 1 file changed, 50 insertions(+), 11 deletions(-) diff --git a/crates/stages/src/test_utils/macros.rs b/crates/stages/src/test_utils/macros.rs index bc2c8410b09..e8c6610dfa7 100644 --- a/crates/stages/src/test_utils/macros.rs +++ b/crates/stages/src/test_utils/macros.rs @@ -1,16 +1,22 @@ -// TODO: add comments macro_rules! stage_test_suite { ($runner:ident) => { /// Check that the execution is short-circuited if the database is empty. #[tokio::test] async fn execute_empty_db() { + // Set up the runner let runner = $runner::default(); + + // Execute the stage with empty database let input = crate::stage::ExecInput::default(); + + // Run stage execution let result = runner.execute(input).await.unwrap(); assert_matches!( result, Err(crate::error::StageError::DatabaseIntegrity(_)) ); + + // Validate the stage execution assert!(runner.validate_execution(input, result.ok()).is_ok(), "execution validation"); } @@ -18,26 +24,39 @@ macro_rules! stage_test_suite { #[tokio::test] async fn execute_already_reached_target() { let stage_progress = 1000; + + // Set up the runner let mut runner = $runner::default(); let input = crate::stage::ExecInput { previous_stage: Some((crate::test_utils::PREV_STAGE_ID, stage_progress)), stage_progress: Some(stage_progress), }; let seed = runner.seed_execution(input).expect("failed to seed"); + + // Run stage execution let rx = runner.execute(input); + + // Run `after_execution` hook runner.after_execution(seed).await.expect("failed to run after execution hook"); + + // Assert the successful result let result = rx.await.unwrap(); assert_matches!( result, Ok(ExecOutput { done, reached_tip, stage_progress }) if done && reached_tip && stage_progress == stage_progress ); + + // Validate the stage execution assert!(runner.validate_execution(input, result.ok()).is_ok(), "execution validation"); } + // Run the complete stage execution flow. #[tokio::test] async fn execute() { let (previous_stage, stage_progress) = (1000, 100); + + // Set up the runner let mut runner = $runner::default(); let input = crate::stage::ExecInput { previous_stage: Some((crate::test_utils::PREV_STAGE_ID, previous_stage)), @@ -45,43 +64,58 @@ macro_rules! stage_test_suite { }; let seed = runner.seed_execution(input).expect("failed to seed"); let rx = runner.execute(input); + + // Run `after_execution` hook runner.after_execution(seed).await.expect("failed to run after execution hook"); + + // Assert the successful result let result = rx.await.unwrap(); - println!("RESULT >>> {:?}", result.unwrap_err().to_string()); - // assert_matches!( - // result, - // Ok(ExecOutput { done, reached_tip, stage_progress }) - // if done && reached_tip && stage_progress == previous_stage - // ); - // assert!(runner.validate_execution(input, result.ok()).is_ok(), "execution validation"); + assert_matches!( + result, + Ok(ExecOutput { done, reached_tip, stage_progress }) + if done && reached_tip && stage_progress == previous_stage + ); + + // Validate the stage execution + assert!(runner.validate_execution(input, result.ok()).is_ok(), "execution validation"); } - #[tokio::test] // Check that unwind does not panic on empty database. + #[tokio::test] async fn unwind_empty_db() { + // Set up the runner let runner = $runner::default(); let input = crate::stage::UnwindInput::default(); + + // Run stage unwind let rx = runner.unwind(input); assert_matches!( rx.await.unwrap(), Ok(UnwindOutput { stage_progress }) if stage_progress == input.unwind_to ); + + // Validate the stage unwind assert!(runner.validate_unwind(input).is_ok(), "unwind validation"); } + // Run complete execute and unwind flow. #[tokio::test] async fn unwind() { let (previous_stage, stage_progress) = (1000, 100); - let mut runner = $runner::default(); - // Run execute + // Set up the runner + let mut runner = $runner::default(); let execute_input = crate::stage::ExecInput { previous_stage: Some((crate::test_utils::PREV_STAGE_ID, previous_stage)), stage_progress: Some(stage_progress), }; let seed = runner.seed_execution(execute_input).expect("failed to seed"); + + // Run stage execution let rx = runner.execute(execute_input); runner.after_execution(seed).await.expect("failed to run after execution hook"); + + // Assert the successful execution result let result = rx.await.unwrap(); assert_matches!( result, @@ -90,14 +124,19 @@ macro_rules! stage_test_suite { ); assert!(runner.validate_execution(execute_input, result.ok()).is_ok(), "execution validation"); + // Run stage unwind let unwind_input = crate::stage::UnwindInput { unwind_to: stage_progress, stage_progress, bad_block: None, }; let rx = runner.unwind(unwind_input); + + // Assert the successful unwind result assert_matches!( rx.await.unwrap(), Ok(UnwindOutput { stage_progress }) if stage_progress == unwind_input.unwind_to ); + + // Validate the stage unwind assert!(runner.validate_unwind(unwind_input).is_ok(), "unwind validation"); } }; From c473012b1eeb11d1602b6c918ceb9c82f52226ee Mon Sep 17 00:00:00 2001 From: Roman Krasiuk Date: Thu, 17 Nov 2022 16:23:13 +0200 Subject: [PATCH 07/17] fmt --- crates/stages/src/stages/bodies.rs | 70 +++++++++++++++++------------- 1 file changed, 39 insertions(+), 31 deletions(-) diff --git a/crates/stages/src/stages/bodies.rs b/crates/stages/src/stages/bodies.rs index c349d0ecc4d..a43c7026781 100644 --- a/crates/stages/src/stages/bodies.rs +++ b/crates/stages/src/stages/bodies.rs @@ -232,12 +232,12 @@ impl BodyStage { #[cfg(test)] mod tests { use super::*; - use crate::test_utils::{ExecuteStageTestRunner, StageTestRunner, UnwindStageTestRunner, PREV_STAGE_ID, stage_test_suite}; - use assert_matches::assert_matches; - use reth_interfaces::{ - consensus, - p2p::bodies::error::DownloadError, + use crate::test_utils::{ + stage_test_suite, ExecuteStageTestRunner, StageTestRunner, UnwindStageTestRunner, + PREV_STAGE_ID, }; + use assert_matches::assert_matches; + use reth_interfaces::{consensus, p2p::bodies::error::DownloadError}; use std::collections::HashMap; use test_utils::*; @@ -411,12 +411,15 @@ mod tests { runner.validate_db_blocks(stage_progress).expect("Written block data invalid"); // Delete a transaction - runner.db().commit(|tx| { - let mut tx_cursor = tx.cursor_mut::()?; - tx_cursor.last()?.expect("Could not read last transaction"); - tx_cursor.delete_current()?; - Ok(()) - }).expect("Could not delete a transaction"); + runner + .db() + .commit(|tx| { + let mut tx_cursor = tx.cursor_mut::()?; + tx_cursor.last()?.expect("Could not read last transaction"); + tx_cursor.delete_current()?; + Ok(()) + }) + .expect("Could not delete a transaction"); // Unwind all of it let unwind_to = 1; @@ -464,9 +467,10 @@ mod tests { use crate::{ stages::bodies::BodyStage, test_utils::{ - ExecuteStageTestRunner, StageTestDB, StageTestRunner, UnwindStageTestRunner, TestRunnerError, + ExecuteStageTestRunner, StageTestDB, StageTestRunner, TestRunnerError, + UnwindStageTestRunner, }, - ExecInput, UnwindInput, ExecOutput, + ExecInput, ExecOutput, UnwindInput, }; use assert_matches::assert_matches; use reth_eth_wire::BlockBody; @@ -480,12 +484,12 @@ mod tests { downloader::{BodiesStream, BodyDownloader}, error::{BodiesClientError, DownloadError}, }, - test_utils::{TestConsensus, generators::random_block_range}, + test_utils::{generators::random_block_range, TestConsensus}, }; use reth_primitives::{ BigEndianHash, BlockLocked, BlockNumber, Header, SealedHeader, H256, U256, }; - use std::{collections::HashMap, ops::Deref, time::Duration, sync::Arc}; + use std::{collections::HashMap, ops::Deref, sync::Arc, time::Duration}; /// The block hash of the genesis block. pub(crate) const GENESIS_HASH: H256 = H256::zero(); @@ -527,7 +531,10 @@ mod tests { self.batch_size = batch_size; } - pub(crate) fn set_responses(&mut self, responses: HashMap>) { + pub(crate) fn set_responses( + &mut self, + responses: HashMap>, + ) { self.responses = responses; } } @@ -552,10 +559,7 @@ mod tests { impl ExecuteStageTestRunner for BodyTestRunner { type Seed = Vec; - fn seed_execution( - &mut self, - input: ExecInput, - ) -> Result { + fn seed_execution(&mut self, input: ExecInput) -> Result { let start = input.stage_progress.unwrap_or_default(); let end = input.previous_stage.as_ref().map(|(_, num)| *num).unwrap_or_default(); let blocks = random_block_range(start..end, GENESIS_HASH); @@ -579,14 +583,14 @@ mod tests { } impl UnwindStageTestRunner for BodyTestRunner { - fn validate_unwind( - &self, - input: UnwindInput, - ) -> Result<(), TestRunnerError> { - self.db.check_no_entry_above::(input.unwind_to, |key| key.number())?; + fn validate_unwind(&self, input: UnwindInput) -> Result<(), TestRunnerError> { + self.db.check_no_entry_above::(input.unwind_to, |key| { + key.number() + })?; if let Some(last_body) = self.last_body() { let last_tx_id = last_body.base_tx_id + last_body.tx_amount; - self.db.check_no_entry_above::(last_tx_id, |key| key)?; + self.db + .check_no_entry_above::(last_tx_id, |key| key)?; } Ok(()) } @@ -610,7 +614,10 @@ mod tests { } /// Insert header into tables - pub(crate) fn insert_header(&self, header: &SealedHeader) -> Result<(), TestRunnerError> { + pub(crate) fn insert_header( + &self, + header: &SealedHeader, + ) -> Result<(), TestRunnerError> { self.insert_headers(std::iter::once(header))?; Ok(()) } @@ -657,7 +664,7 @@ mod tests { self.db.query(|tx| { let mut block_body_cursor = tx.cursor::()?; let mut transaction_cursor = tx.cursor::()?; - + let mut entry = block_body_cursor.first()?; let mut prev_max_tx_id = 0; while let Some((key, body)) = entry { @@ -666,7 +673,7 @@ mod tests { "We wrote a block body outside of our synced range. Found block with number {}, highest block according to stage is {}", key.number(), highest_block ); - + assert!(prev_max_tx_id == body.base_tx_id, "Transaction IDs are malformed."); for num in 0..body.tx_amount { let tx_id = body.base_tx_id + num; @@ -679,7 +686,7 @@ mod tests { prev_max_tx_id = body.base_tx_id + body.tx_amount; entry = block_body_cursor.next()?; } - + Ok(()) })?; Ok(()) @@ -730,7 +737,8 @@ mod tests { { Box::pin(futures_util::stream::iter(hashes.into_iter().map( |(block_number, hash)| { - let result = self.responses + let result = self + .responses .get(hash) .expect("Stage tried downloading a block we do not have.") .clone()?; From 9147e8653d538e484417d3b5045c263b7e3622c5 Mon Sep 17 00:00:00 2001 From: Roman Krasiuk Date: Thu, 17 Nov 2022 16:33:58 +0200 Subject: [PATCH 08/17] cleanup dup code --- crates/stages/src/stages/bodies.rs | 53 ++++-------------------- crates/stages/src/stages/headers.rs | 40 ++---------------- crates/stages/src/test_utils/stage_db.rs | 32 ++++++++++++-- 3 files changed, 39 insertions(+), 86 deletions(-) diff --git a/crates/stages/src/stages/bodies.rs b/crates/stages/src/stages/bodies.rs index a43c7026781..b62a872a24b 100644 --- a/crates/stages/src/stages/bodies.rs +++ b/crates/stages/src/stages/bodies.rs @@ -475,10 +475,7 @@ mod tests { use assert_matches::assert_matches; use reth_eth_wire::BlockBody; use reth_interfaces::{ - db::{ - models::{BlockNumHash, StoredBlockBody}, - tables, DbCursorRO, DbTx, DbTxMut, - }, + db::{models::StoredBlockBody, tables, DbCursorRO, DbTx, DbTxMut}, p2p::bodies::{ client::BodiesClient, downloader::{BodiesStream, BodyDownloader}, @@ -486,10 +483,8 @@ mod tests { }, test_utils::{generators::random_block_range, TestConsensus}, }; - use reth_primitives::{ - BigEndianHash, BlockLocked, BlockNumber, Header, SealedHeader, H256, U256, - }; - use std::{collections::HashMap, ops::Deref, sync::Arc, time::Duration}; + use reth_primitives::{BlockLocked, BlockNumber, Header, SealedHeader, H256}; + use std::{collections::HashMap, sync::Arc, time::Duration}; /// The block hash of the genesis block. pub(crate) const GENESIS_HASH: H256 = H256::zero(); @@ -564,7 +559,7 @@ mod tests { let end = input.previous_stage.as_ref().map(|(_, num)| *num).unwrap_or_default(); let blocks = random_block_range(start..end, GENESIS_HASH); self.insert_genesis()?; - self.insert_headers(blocks.iter().map(|block| &block.header))?; + self.db.insert_headers(blocks.iter().map(|block| &block.header))?; self.set_responses(blocks.iter().map(body_by_hash).collect()); Ok(blocks) } @@ -602,7 +597,8 @@ mod tests { /// The genesis block always has no transactions and no ommers, and it always has the /// same hash. pub(crate) fn insert_genesis(&self) -> Result<(), TestRunnerError> { - self.insert_header(&SealedHeader::new(Header::default(), GENESIS_HASH))?; + let header = SealedHeader::new(Header::default(), GENESIS_HASH); + self.db.insert_headers(std::iter::once(&header))?; self.db.commit(|tx| { tx.put::( (0, GENESIS_HASH).into(), @@ -613,42 +609,7 @@ mod tests { Ok(()) } - /// Insert header into tables - pub(crate) fn insert_header( - &self, - header: &SealedHeader, - ) -> Result<(), TestRunnerError> { - self.insert_headers(std::iter::once(header))?; - Ok(()) - } - - /// Insert headers into tables - /// TODO: move to common inserter - pub(crate) fn insert_headers<'a, I>(&self, headers: I) -> Result<(), TestRunnerError> - where - I: Iterator, - { - let headers = headers.collect::>(); - self.db - .map_put::(&headers, |h| (h.hash(), h.number))?; - self.db.map_put::(&headers, |h| { - (BlockNumHash((h.number, h.hash())), h.deref().clone().unseal()) - })?; - self.db.map_put::(&headers, |h| { - (h.number, h.hash()) - })?; - - self.db.transform_append::(&headers, |prev, h| { - let prev_td = U256::from_big_endian(&prev.clone().unwrap_or_default()); - ( - BlockNumHash((h.number, h.hash())), - H256::from_uint(&(prev_td + h.difficulty)).as_bytes().to_vec(), - ) - })?; - - Ok(()) - } - + /// Retrieve the last body from the database pub(crate) fn last_body(&self) -> Option { self.db .query(|tx| Ok(tx.cursor::()?.last()?.map(|e| e.1))) diff --git a/crates/stages/src/stages/headers.rs b/crates/stages/src/stages/headers.rs index 49039a4c0cf..ce6a2320b6f 100644 --- a/crates/stages/src/stages/headers.rs +++ b/crates/stages/src/stages/headers.rs @@ -281,8 +281,8 @@ mod tests { TestConsensus, TestHeaderDownloader, TestHeadersClient, }, }; - use reth_primitives::{rpc::BigEndianHash, BlockNumber, SealedHeader, H256, U256}; - use std::{ops::Deref, sync::Arc}; + use reth_primitives::{BlockNumber, SealedHeader, H256, U256}; + use std::sync::Arc; pub(crate) struct HeadersTestRunner { pub(crate) consensus: Arc, @@ -327,7 +327,7 @@ mod tests { fn seed_execution(&mut self, input: ExecInput) -> Result { let start = input.stage_progress.unwrap_or_default(); let head = random_header(start, None); - self.insert_header(&head)?; + self.db.insert_headers(std::iter::once(&head))?; // use previous progress as seed size let end = input.previous_stage.map(|(_, num)| num).unwrap_or_default() + 1; @@ -425,40 +425,6 @@ mod tests { } impl HeadersTestRunner { - /// Insert header into tables - pub(crate) fn insert_header( - &self, - header: &SealedHeader, - ) -> Result<(), TestRunnerError> { - self.insert_headers(std::iter::once(header)) - } - - /// Insert headers into tables - pub(crate) fn insert_headers<'a, I>(&self, headers: I) -> Result<(), TestRunnerError> - where - I: Iterator, - { - let headers = headers.collect::>(); - self.db - .map_put::(&headers, |h| (h.hash(), h.number))?; - self.db.map_put::(&headers, |h| { - (BlockNumHash((h.number, h.hash())), h.deref().clone().unseal()) - })?; - self.db.map_put::(&headers, |h| { - (h.number, h.hash()) - })?; - - self.db.transform_append::(&headers, |prev, h| { - let prev_td = U256::from_big_endian(&prev.clone().unwrap_or_default()); - ( - BlockNumHash((h.number, h.hash())), - H256::from_uint(&(prev_td + h.difficulty)).as_bytes().to_vec(), - ) - })?; - - Ok(()) - } - pub(crate) fn check_no_header_entry_above( &self, block: BlockNumber, diff --git a/crates/stages/src/test_utils/stage_db.rs b/crates/stages/src/test_utils/stage_db.rs index f14b1cb4f2f..9dd0c8dc979 100644 --- a/crates/stages/src/test_utils/stage_db.rs +++ b/crates/stages/src/test_utils/stage_db.rs @@ -2,9 +2,11 @@ use reth_db::{ kv::{test_utils::create_test_db, tx::Tx, Env, EnvKind}, mdbx::{WriteMap, RW}, }; -use reth_interfaces::db::{self, DBContainer, DbCursorRO, DbCursorRW, DbTx, DbTxMut, Table}; -use reth_primitives::BlockNumber; -use std::{borrow::Borrow, sync::Arc}; +use reth_interfaces::db::{ + self, models::BlockNumHash, tables, DBContainer, DbCursorRO, DbCursorRW, DbTx, DbTxMut, Table, +}; +use reth_primitives::{BigEndianHash, BlockNumber, SealedHeader, H256, U256}; +use std::{borrow::Borrow, ops::Deref, sync::Arc}; /// The [StageTestDB] is used as an internal /// database for testing stage implementation. @@ -146,4 +148,28 @@ impl StageTestDB { Ok(()) }) } + + /// Insert ordered collection of [SealedHeader] into the corresponding tables + /// that are supposed to be populated by the headers stage. + pub(crate) fn insert_headers<'a, I>(&self, headers: I) -> Result<(), db::Error> + where + I: Iterator, + { + let headers = headers.collect::>(); + self.map_put::(&headers, |h| (h.hash(), h.number))?; + self.map_put::(&headers, |h| { + (BlockNumHash((h.number, h.hash())), h.deref().clone().unseal()) + })?; + self.map_put::(&headers, |h| (h.number, h.hash()))?; + + self.transform_append::(&headers, |prev, h| { + let prev_td = U256::from_big_endian(&prev.clone().unwrap_or_default()); + ( + BlockNumHash((h.number, h.hash())), + H256::from_uint(&(prev_td + h.difficulty)).as_bytes().to_vec(), + ) + })?; + + Ok(()) + } } From 2af4d766291d21fe67e5f5b8b2841b4fea55fe56 Mon Sep 17 00:00:00 2001 From: Roman Krasiuk Date: Thu, 17 Nov 2022 16:45:57 +0200 Subject: [PATCH 09/17] cleanup insert_headers helper fn --- crates/stages/src/test_utils/stage_db.rs | 36 +++++++++++++----------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/crates/stages/src/test_utils/stage_db.rs b/crates/stages/src/test_utils/stage_db.rs index 9dd0c8dc979..f37d4038b00 100644 --- a/crates/stages/src/test_utils/stage_db.rs +++ b/crates/stages/src/test_utils/stage_db.rs @@ -6,7 +6,7 @@ use reth_interfaces::db::{ self, models::BlockNumHash, tables, DBContainer, DbCursorRO, DbCursorRW, DbTx, DbTxMut, Table, }; use reth_primitives::{BigEndianHash, BlockNumber, SealedHeader, H256, U256}; -use std::{borrow::Borrow, ops::Deref, sync::Arc}; +use std::{borrow::Borrow, sync::Arc}; /// The [StageTestDB] is used as an internal /// database for testing stage implementation. @@ -155,21 +155,25 @@ impl StageTestDB { where I: Iterator, { - let headers = headers.collect::>(); - self.map_put::(&headers, |h| (h.hash(), h.number))?; - self.map_put::(&headers, |h| { - (BlockNumHash((h.number, h.hash())), h.deref().clone().unseal()) - })?; - self.map_put::(&headers, |h| (h.number, h.hash()))?; - - self.transform_append::(&headers, |prev, h| { - let prev_td = U256::from_big_endian(&prev.clone().unwrap_or_default()); - ( - BlockNumHash((h.number, h.hash())), - H256::from_uint(&(prev_td + h.difficulty)).as_bytes().to_vec(), - ) - })?; + self.commit(|tx| { + let headers = headers.collect::>(); - Ok(()) + let mut td = U256::from_big_endian( + &tx.cursor::()?.last()?.map(|(_, v)| v).unwrap_or_default(), + ); + + for header in headers { + let key: BlockNumHash = (header.number, header.hash()).into(); + + tx.put::(header.number, header.hash())?; + tx.put::(header.hash(), header.number)?; + tx.put::(key, header.clone().unseal())?; + + td += header.difficulty; + tx.put::(key, H256::from_uint(&td).as_bytes().to_vec())?; + } + + Ok(()) + }) } } From 1db7a8659b52480533184e138cee8d3863cd46ea Mon Sep 17 00:00:00 2001 From: Roman Krasiuk Date: Fri, 18 Nov 2022 10:20:35 +0200 Subject: [PATCH 10/17] fix tests --- crates/db/src/kv/mod.rs | 2 +- crates/stages/src/stages/bodies.rs | 34 +++++++++++--------------- crates/stages/src/stages/headers.rs | 2 +- crates/stages/src/test_utils/macros.rs | 4 +-- 4 files changed, 18 insertions(+), 24 deletions(-) diff --git a/crates/db/src/kv/mod.rs b/crates/db/src/kv/mod.rs index a5600702e8f..01186c0940a 100644 --- a/crates/db/src/kv/mod.rs +++ b/crates/db/src/kv/mod.rs @@ -61,7 +61,7 @@ impl Env { inner: Environment::new() .set_max_dbs(TABLES.len()) .set_geometry(Geometry { - size: Some(0..0x100000), // TODO: reevaluate + size: Some(0..0x1000000), // TODO: reevaluate growth_step: Some(0x100000), // TODO: reevaluate shrink_threshold: None, page_size: Some(PageSize::Set(default_page_size())), diff --git a/crates/stages/src/stages/bodies.rs b/crates/stages/src/stages/bodies.rs index b62a872a24b..cd6a71330a6 100644 --- a/crates/stages/src/stages/bodies.rs +++ b/crates/stages/src/stages/bodies.rs @@ -81,6 +81,9 @@ impl Stage for BodyStage Stage for BodyStage Result { let start = input.stage_progress.unwrap_or_default(); - let end = input.previous_stage.as_ref().map(|(_, num)| *num).unwrap_or_default(); + let end = + input.previous_stage.as_ref().map(|(_, num)| *num + 1).unwrap_or_default(); let blocks = random_block_range(start..end, GENESIS_HASH); self.insert_genesis()?; self.db.insert_headers(blocks.iter().map(|block| &block.header))?; diff --git a/crates/stages/src/stages/headers.rs b/crates/stages/src/stages/headers.rs index ce6a2320b6f..52abdf4884c 100644 --- a/crates/stages/src/stages/headers.rs +++ b/crates/stages/src/stages/headers.rs @@ -58,8 +58,8 @@ impl Stage(tx, last_block_num).await?; + // TODO: add batch size // download the headers - // TODO: handle input.max_block let last_hash = tx .get::(last_block_num)? .ok_or(DatabaseIntegrityError::CanonicalHash { number: last_block_num })?; diff --git a/crates/stages/src/test_utils/macros.rs b/crates/stages/src/test_utils/macros.rs index e8c6610dfa7..0bb0ccdc8cf 100644 --- a/crates/stages/src/test_utils/macros.rs +++ b/crates/stages/src/test_utils/macros.rs @@ -54,7 +54,7 @@ macro_rules! stage_test_suite { // Run the complete stage execution flow. #[tokio::test] async fn execute() { - let (previous_stage, stage_progress) = (1000, 100); + let (previous_stage, stage_progress) = (500, 100); // Set up the runner let mut runner = $runner::default(); @@ -101,7 +101,7 @@ macro_rules! stage_test_suite { // Run complete execute and unwind flow. #[tokio::test] async fn unwind() { - let (previous_stage, stage_progress) = (1000, 100); + let (previous_stage, stage_progress) = (500, 100); // Set up the runner let mut runner = $runner::default(); From c695d2bc901a1528df0494ff721772cf32c10c7d Mon Sep 17 00:00:00 2001 From: Roman Krasiuk Date: Fri, 18 Nov 2022 10:29:08 +0200 Subject: [PATCH 11/17] linter --- crates/net/headers-downloaders/src/linear.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/net/headers-downloaders/src/linear.rs b/crates/net/headers-downloaders/src/linear.rs index b2a2d34ca03..1dc2dae6d92 100644 --- a/crates/net/headers-downloaders/src/linear.rs +++ b/crates/net/headers-downloaders/src/linear.rs @@ -215,7 +215,7 @@ mod tests { static CONSENSUS: Lazy> = Lazy::new(|| Arc::new(TestConsensus::default())); static CONSENSUS_FAIL: Lazy> = Lazy::new(|| { - let mut consensus = TestConsensus::default(); + let consensus = TestConsensus::default(); consensus.set_fail_validation(true); Arc::new(consensus) }); From 8b6d457a194413b4d4911f84369a5bd0068e1333 Mon Sep 17 00:00:00 2001 From: Roman Krasiuk Date: Fri, 18 Nov 2022 12:44:23 +0200 Subject: [PATCH 12/17] switch mutex to atomic --- crates/interfaces/src/test_utils/headers.rs | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/crates/interfaces/src/test_utils/headers.rs b/crates/interfaces/src/test_utils/headers.rs index 4945627e379..6e4369e4650 100644 --- a/crates/interfaces/src/test_utils/headers.rs +++ b/crates/interfaces/src/test_utils/headers.rs @@ -11,7 +11,10 @@ use reth_primitives::{BlockLocked, Header, SealedHeader, H256, H512}; use reth_rpc_types::engine::ForkchoiceState; use std::{ collections::HashSet, - sync::{Arc, Mutex, MutexGuard}, + sync::{ + atomic::{AtomicBool, Ordering}, + Arc, + }, time::Duration, }; use tokio::sync::{broadcast, mpsc, watch}; @@ -150,7 +153,7 @@ pub struct TestConsensus { /// Watcher over the forkchoice state channel: (watch::Sender, watch::Receiver), /// Flag whether the header validation should purposefully fail - fail_validation: Mutex, + fail_validation: AtomicBool, // Mutex, } impl Default for TestConsensus { @@ -161,7 +164,7 @@ impl Default for TestConsensus { finalized_block_hash: H256::zero(), safe_block_hash: H256::zero(), }), - fail_validation: Mutex::new(false), + fail_validation: AtomicBool::new(false), // Mutex::new(false), } } } @@ -177,14 +180,14 @@ impl TestConsensus { self.channel.0.send(state).expect("updating fork choice state failed"); } - /// Acquire lock on failed validation flag - pub fn fail_validation(&self) -> MutexGuard<'_, bool> { - self.fail_validation.lock().expect("failed to acquite consensus mutex") + /// Get the failed validation flag + pub fn fail_validation(&self) -> bool { + self.fail_validation.load(Ordering::SeqCst) } /// Update the validation flag pub fn set_fail_validation(&self, val: bool) { - *self.fail_validation() = val; + self.fail_validation.store(val, Ordering::SeqCst) } } @@ -199,7 +202,7 @@ impl Consensus for TestConsensus { _header: &SealedHeader, _parent: &SealedHeader, ) -> Result<(), consensus::Error> { - if *self.fail_validation() { + if self.fail_validation() { Err(consensus::Error::BaseFeeMissing) } else { Ok(()) @@ -207,7 +210,7 @@ impl Consensus for TestConsensus { } fn pre_validate_block(&self, _block: &BlockLocked) -> Result<(), consensus::Error> { - if *self.fail_validation() { + if self.fail_validation() { Err(consensus::Error::BaseFeeMissing) } else { Ok(()) From cc3340d7d34d3719f7968150fc58aa2df1fb380d Mon Sep 17 00:00:00 2001 From: Roman Krasiuk Date: Fri, 18 Nov 2022 12:45:26 +0200 Subject: [PATCH 13/17] cleanup --- crates/interfaces/src/test_utils/headers.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/interfaces/src/test_utils/headers.rs b/crates/interfaces/src/test_utils/headers.rs index 6e4369e4650..087594c0922 100644 --- a/crates/interfaces/src/test_utils/headers.rs +++ b/crates/interfaces/src/test_utils/headers.rs @@ -153,7 +153,7 @@ pub struct TestConsensus { /// Watcher over the forkchoice state channel: (watch::Sender, watch::Receiver), /// Flag whether the header validation should purposefully fail - fail_validation: AtomicBool, // Mutex, + fail_validation: AtomicBool, } impl Default for TestConsensus { @@ -164,7 +164,7 @@ impl Default for TestConsensus { finalized_block_hash: H256::zero(), safe_block_hash: H256::zero(), }), - fail_validation: AtomicBool::new(false), // Mutex::new(false), + fail_validation: AtomicBool::new(false), } } } From 14e1800483e042ad528d42b80f68f01b1d7d992c Mon Sep 17 00:00:00 2001 From: Roman Krasiuk Date: Fri, 18 Nov 2022 15:12:57 +0200 Subject: [PATCH 14/17] revert --- crates/stages/src/stages/bodies.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/stages/src/stages/bodies.rs b/crates/stages/src/stages/bodies.rs index cd6a71330a6..b604e9df4c5 100644 --- a/crates/stages/src/stages/bodies.rs +++ b/crates/stages/src/stages/bodies.rs @@ -92,7 +92,7 @@ impl Stage for BodyStage Date: Fri, 18 Nov 2022 15:25:59 -0800 Subject: [PATCH 15/17] test: make unwind runner return value instead of channel --- crates/stages/src/stages/bodies.rs | 4 ++-- crates/stages/src/test_utils/macros.rs | 8 ++++---- crates/stages/src/test_utils/runner.rs | 7 ++++--- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/crates/stages/src/stages/bodies.rs b/crates/stages/src/stages/bodies.rs index b604e9df4c5..e5004173432 100644 --- a/crates/stages/src/stages/bodies.rs +++ b/crates/stages/src/stages/bodies.rs @@ -417,9 +417,9 @@ mod tests { // Unwind all of it let unwind_to = 1; let input = UnwindInput { bad_block: None, stage_progress, unwind_to }; - let rx = runner.unwind(input); + let res = runner.unwind(input).await; assert_matches!( - rx.await.unwrap(), + res, Ok(UnwindOutput { stage_progress }) if stage_progress == 1 ); diff --git a/crates/stages/src/test_utils/macros.rs b/crates/stages/src/test_utils/macros.rs index 0bb0ccdc8cf..f92c96c7dc1 100644 --- a/crates/stages/src/test_utils/macros.rs +++ b/crates/stages/src/test_utils/macros.rs @@ -88,9 +88,9 @@ macro_rules! stage_test_suite { let input = crate::stage::UnwindInput::default(); // Run stage unwind - let rx = runner.unwind(input); + let rx = runner.unwind(input).await; assert_matches!( - rx.await.unwrap(), + rx, Ok(UnwindOutput { stage_progress }) if stage_progress == input.unwind_to ); @@ -128,11 +128,11 @@ macro_rules! stage_test_suite { let unwind_input = crate::stage::UnwindInput { unwind_to: stage_progress, stage_progress, bad_block: None, }; - let rx = runner.unwind(unwind_input); + let rx = runner.unwind(unwind_input).await; // Assert the successful unwind result assert_matches!( - rx.await.unwrap(), + rx, Ok(UnwindOutput { stage_progress }) if stage_progress == unwind_input.unwind_to ); diff --git a/crates/stages/src/test_utils/runner.rs b/crates/stages/src/test_utils/runner.rs index c81574dbd5f..761e9309599 100644 --- a/crates/stages/src/test_utils/runner.rs +++ b/crates/stages/src/test_utils/runner.rs @@ -59,15 +59,16 @@ pub(crate) trait ExecuteStageTestRunner: StageTestRunner { } } +#[async_trait::async_trait] pub(crate) trait UnwindStageTestRunner: StageTestRunner { /// Validate the unwind fn validate_unwind(&self, input: UnwindInput) -> Result<(), TestRunnerError>; /// Run [Stage::unwind] and return a receiver for the result. - fn unwind( + async fn unwind( &self, input: UnwindInput, - ) -> oneshot::Receiver>> { + ) -> Result> { let (tx, rx) = oneshot::channel(); let (db, mut stage) = (self.db().inner(), self.stage()); tokio::spawn(async move { @@ -76,6 +77,6 @@ pub(crate) trait UnwindStageTestRunner: StageTestRunner { db.commit().expect("failed to commit"); tx.send(result).expect("failed to send result"); }); - rx + Box::pin(rx).await.unwrap() } } From f8608654f2e4cf97f60ce6aa95c28009f71d5331 Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Fri, 18 Nov 2022 15:37:44 -0800 Subject: [PATCH 16/17] test: make execute runner return value instead of channel --- crates/stages/src/stages/bodies.rs | 39 +++++++++++--------------- crates/stages/src/stages/headers.rs | 9 ++---- crates/stages/src/test_utils/macros.rs | 11 +++----- crates/stages/src/test_utils/runner.rs | 4 +-- 4 files changed, 26 insertions(+), 37 deletions(-) diff --git a/crates/stages/src/stages/bodies.rs b/crates/stages/src/stages/bodies.rs index e5004173432..83506909327 100644 --- a/crates/stages/src/stages/bodies.rs +++ b/crates/stages/src/stages/bodies.rs @@ -264,16 +264,15 @@ mod tests { runner.set_batch_size(10); // Run the stage - let rx = runner.execute(input); + let result = runner.execute(input).await; // Check that we only synced around `batch_size` blocks even though the number of blocks // synced by the previous stage is higher - let output = rx.await.unwrap(); assert_matches!( - output, + result, Ok(ExecOutput { stage_progress, reached_tip: true, done: false }) if stage_progress < 200 ); - assert!(runner.validate_execution(input, output.ok()).is_ok(), "execution validation"); + assert!(runner.validate_execution(input, result.ok()).is_ok(), "execution validation"); } /// Same as [partial_body_download] except the `batch_size` is not hit. @@ -293,16 +292,15 @@ mod tests { runner.set_batch_size(40); // Run the stage - let rx = runner.execute(input); + let result = runner.execute(input).await; // Check that we synced all blocks successfully, even though our `batch_size` allows us to // sync more (if there were more headers) - let output = rx.await.unwrap(); assert_matches!( - output, + result, Ok(ExecOutput { stage_progress: 20, reached_tip: true, done: true }) ); - assert!(runner.validate_execution(input, output.ok()).is_ok(), "execution validation"); + assert!(runner.validate_execution(input, result.ok()).is_ok(), "execution validation"); } /// Same as [full_body_download] except we have made progress before @@ -321,10 +319,9 @@ mod tests { runner.set_batch_size(10); // Run the stage - let rx = runner.execute(input); + let first_run = runner.execute(input).await; // Check that we synced at least 10 blocks - let first_run = rx.await.unwrap(); assert_matches!( first_run, Ok(ExecOutput { stage_progress, reached_tip: true, done: false }) if stage_progress >= 10 @@ -336,15 +333,14 @@ mod tests { previous_stage: Some((PREV_STAGE_ID, previous_stage)), stage_progress: Some(first_run_progress), }; - let rx = runner.execute(input); + let result = runner.execute(input).await; // Check that we synced more blocks - let output = rx.await.unwrap(); assert_matches!( - output, + result, Ok(ExecOutput { stage_progress, reached_tip: true, done: true }) if stage_progress > first_run_progress ); - assert!(runner.validate_execution(input, output.ok()).is_ok(), "execution validation"); + assert!(runner.validate_execution(input, result.ok()).is_ok(), "execution validation"); } /// Checks that the stage asks to unwind if pre-validation of the block fails. @@ -364,11 +360,11 @@ mod tests { runner.consensus.set_fail_validation(true); // Run the stage - let rx = runner.execute(input); + let result = runner.execute(input).await; // Check that the error bubbles up assert_matches!( - rx.await.unwrap(), + result, Err(StageError::Validation { error: consensus::Error::BaseFeeMissing, .. }) ); assert!(runner.validate_execution(input, None).is_ok(), "execution validation"); @@ -391,16 +387,15 @@ mod tests { runner.set_batch_size(40); // Run the stage - let rx = runner.execute(input); + let result = runner.execute(input).await; // Check that we synced all blocks successfully, even though our `batch_size` allows us to // sync more (if there were more headers) - let output = rx.await.unwrap(); assert_matches!( - output, + result, Ok(ExecOutput { stage_progress, reached_tip: true, done: true }) if stage_progress == previous_stage ); - let stage_progress = output.unwrap().stage_progress; + let stage_progress = result.unwrap().stage_progress; runner.validate_db_blocks(stage_progress).expect("Written block data invalid"); // Delete a transaction @@ -449,10 +444,10 @@ mod tests { )])); // Run the stage - let rx = runner.execute(input); + let result = runner.execute(input).await; // Check that the error bubbles up - assert_matches!(rx.await.unwrap(), Err(StageError::Internal(_))); + assert_matches!(result, Err(StageError::Internal(_))); assert!(runner.validate_execution(input, None).is_ok(), "execution validation"); } diff --git a/crates/stages/src/stages/headers.rs b/crates/stages/src/stages/headers.rs index 52abdf4884c..39813bee501 100644 --- a/crates/stages/src/stages/headers.rs +++ b/crates/stages/src/stages/headers.rs @@ -206,9 +206,8 @@ mod tests { let mut runner = HeadersTestRunner::default(); let input = ExecInput::default(); runner.seed_execution(input).expect("failed to seed execution"); - let rx = runner.execute(input); + let result = runner.execute(input).await; runner.consensus.update_tip(H256::from_low_u64_be(1)); - let result = rx.await.unwrap(); assert_matches!( result, Ok(ExecOutput { done: false, reached_tip: false, stage_progress: 0 }) @@ -223,9 +222,8 @@ mod tests { runner.consensus.set_fail_validation(true); let input = ExecInput::default(); let seed = runner.seed_execution(input).expect("failed to seed execution"); - let rx = runner.execute(input); + let result = runner.execute(input).await; runner.after_execution(seed).await.expect("failed to run after execution hook"); - let result = rx.await.unwrap(); assert_matches!(result, Err(StageError::Validation { .. })); assert!(runner.validate_execution(input, result.ok()).is_ok(), "validation failed"); } @@ -240,7 +238,7 @@ mod tests { stage_progress: Some(stage_progress), }; let headers = runner.seed_execution(input).expect("failed to seed execution"); - let rx = runner.execute(input); + let result = runner.execute(input).await; // skip `after_execution` hook for linear downloader let tip = headers.last().unwrap(); @@ -255,7 +253,6 @@ mod tests { }) .await; - let result = rx.await.unwrap(); assert_matches!( result, Ok(ExecOutput { done: true, reached_tip: true, stage_progress }) if stage_progress == tip.number diff --git a/crates/stages/src/test_utils/macros.rs b/crates/stages/src/test_utils/macros.rs index f92c96c7dc1..8ee33ba3cb0 100644 --- a/crates/stages/src/test_utils/macros.rs +++ b/crates/stages/src/test_utils/macros.rs @@ -10,7 +10,7 @@ macro_rules! stage_test_suite { let input = crate::stage::ExecInput::default(); // Run stage execution - let result = runner.execute(input).await.unwrap(); + let result = runner.execute(input).await; assert_matches!( result, Err(crate::error::StageError::DatabaseIntegrity(_)) @@ -34,13 +34,12 @@ macro_rules! stage_test_suite { let seed = runner.seed_execution(input).expect("failed to seed"); // Run stage execution - let rx = runner.execute(input); + let result = runner.execute(input).await; // Run `after_execution` hook runner.after_execution(seed).await.expect("failed to run after execution hook"); // Assert the successful result - let result = rx.await.unwrap(); assert_matches!( result, Ok(ExecOutput { done, reached_tip, stage_progress }) @@ -63,13 +62,12 @@ macro_rules! stage_test_suite { stage_progress: Some(stage_progress), }; let seed = runner.seed_execution(input).expect("failed to seed"); - let rx = runner.execute(input); + let result = runner.execute(input).await; // Run `after_execution` hook runner.after_execution(seed).await.expect("failed to run after execution hook"); // Assert the successful result - let result = rx.await.unwrap(); assert_matches!( result, Ok(ExecOutput { done, reached_tip, stage_progress }) @@ -112,11 +110,10 @@ macro_rules! stage_test_suite { let seed = runner.seed_execution(execute_input).expect("failed to seed"); // Run stage execution - let rx = runner.execute(execute_input); + let result = runner.execute(execute_input).await; runner.after_execution(seed).await.expect("failed to run after execution hook"); // Assert the successful execution result - let result = rx.await.unwrap(); assert_matches!( result, Ok(ExecOutput { done, reached_tip, stage_progress }) diff --git a/crates/stages/src/test_utils/runner.rs b/crates/stages/src/test_utils/runner.rs index 761e9309599..7b59c517fe0 100644 --- a/crates/stages/src/test_utils/runner.rs +++ b/crates/stages/src/test_utils/runner.rs @@ -41,7 +41,7 @@ pub(crate) trait ExecuteStageTestRunner: StageTestRunner { ) -> Result<(), TestRunnerError>; /// Run [Stage::execute] and return a receiver for the result. - fn execute(&self, input: ExecInput) -> oneshot::Receiver> { + async fn execute(&self, input: ExecInput) -> Result { let (tx, rx) = oneshot::channel(); let (db, mut stage) = (self.db().inner(), self.stage()); tokio::spawn(async move { @@ -50,7 +50,7 @@ pub(crate) trait ExecuteStageTestRunner: StageTestRunner { db.commit().expect("failed to commit"); tx.send(result).expect("failed to send message") }); - rx + Box::pin(rx).await.unwrap() } /// Run a hook after [Stage::execute]. Required for Headers & Bodies stages. From deb080737513280f9ccb309050c2e185201177a1 Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Fri, 18 Nov 2022 17:51:48 -0800 Subject: [PATCH 17/17] Revert "test: make execute runner return value instead of channel" This reverts commit f8608654f2e4cf97f60ce6aa95c28009f71d5331. --- crates/stages/src/stages/bodies.rs | 39 +++++++++++++++----------- crates/stages/src/stages/headers.rs | 9 ++++-- crates/stages/src/test_utils/macros.rs | 11 +++++--- crates/stages/src/test_utils/runner.rs | 4 +-- 4 files changed, 37 insertions(+), 26 deletions(-) diff --git a/crates/stages/src/stages/bodies.rs b/crates/stages/src/stages/bodies.rs index 83506909327..e5004173432 100644 --- a/crates/stages/src/stages/bodies.rs +++ b/crates/stages/src/stages/bodies.rs @@ -264,15 +264,16 @@ mod tests { runner.set_batch_size(10); // Run the stage - let result = runner.execute(input).await; + let rx = runner.execute(input); // Check that we only synced around `batch_size` blocks even though the number of blocks // synced by the previous stage is higher + let output = rx.await.unwrap(); assert_matches!( - result, + output, Ok(ExecOutput { stage_progress, reached_tip: true, done: false }) if stage_progress < 200 ); - assert!(runner.validate_execution(input, result.ok()).is_ok(), "execution validation"); + assert!(runner.validate_execution(input, output.ok()).is_ok(), "execution validation"); } /// Same as [partial_body_download] except the `batch_size` is not hit. @@ -292,15 +293,16 @@ mod tests { runner.set_batch_size(40); // Run the stage - let result = runner.execute(input).await; + let rx = runner.execute(input); // Check that we synced all blocks successfully, even though our `batch_size` allows us to // sync more (if there were more headers) + let output = rx.await.unwrap(); assert_matches!( - result, + output, Ok(ExecOutput { stage_progress: 20, reached_tip: true, done: true }) ); - assert!(runner.validate_execution(input, result.ok()).is_ok(), "execution validation"); + assert!(runner.validate_execution(input, output.ok()).is_ok(), "execution validation"); } /// Same as [full_body_download] except we have made progress before @@ -319,9 +321,10 @@ mod tests { runner.set_batch_size(10); // Run the stage - let first_run = runner.execute(input).await; + let rx = runner.execute(input); // Check that we synced at least 10 blocks + let first_run = rx.await.unwrap(); assert_matches!( first_run, Ok(ExecOutput { stage_progress, reached_tip: true, done: false }) if stage_progress >= 10 @@ -333,14 +336,15 @@ mod tests { previous_stage: Some((PREV_STAGE_ID, previous_stage)), stage_progress: Some(first_run_progress), }; - let result = runner.execute(input).await; + let rx = runner.execute(input); // Check that we synced more blocks + let output = rx.await.unwrap(); assert_matches!( - result, + output, Ok(ExecOutput { stage_progress, reached_tip: true, done: true }) if stage_progress > first_run_progress ); - assert!(runner.validate_execution(input, result.ok()).is_ok(), "execution validation"); + assert!(runner.validate_execution(input, output.ok()).is_ok(), "execution validation"); } /// Checks that the stage asks to unwind if pre-validation of the block fails. @@ -360,11 +364,11 @@ mod tests { runner.consensus.set_fail_validation(true); // Run the stage - let result = runner.execute(input).await; + let rx = runner.execute(input); // Check that the error bubbles up assert_matches!( - result, + rx.await.unwrap(), Err(StageError::Validation { error: consensus::Error::BaseFeeMissing, .. }) ); assert!(runner.validate_execution(input, None).is_ok(), "execution validation"); @@ -387,15 +391,16 @@ mod tests { runner.set_batch_size(40); // Run the stage - let result = runner.execute(input).await; + let rx = runner.execute(input); // Check that we synced all blocks successfully, even though our `batch_size` allows us to // sync more (if there were more headers) + let output = rx.await.unwrap(); assert_matches!( - result, + output, Ok(ExecOutput { stage_progress, reached_tip: true, done: true }) if stage_progress == previous_stage ); - let stage_progress = result.unwrap().stage_progress; + let stage_progress = output.unwrap().stage_progress; runner.validate_db_blocks(stage_progress).expect("Written block data invalid"); // Delete a transaction @@ -444,10 +449,10 @@ mod tests { )])); // Run the stage - let result = runner.execute(input).await; + let rx = runner.execute(input); // Check that the error bubbles up - assert_matches!(result, Err(StageError::Internal(_))); + assert_matches!(rx.await.unwrap(), Err(StageError::Internal(_))); assert!(runner.validate_execution(input, None).is_ok(), "execution validation"); } diff --git a/crates/stages/src/stages/headers.rs b/crates/stages/src/stages/headers.rs index 39813bee501..52abdf4884c 100644 --- a/crates/stages/src/stages/headers.rs +++ b/crates/stages/src/stages/headers.rs @@ -206,8 +206,9 @@ mod tests { let mut runner = HeadersTestRunner::default(); let input = ExecInput::default(); runner.seed_execution(input).expect("failed to seed execution"); - let result = runner.execute(input).await; + let rx = runner.execute(input); runner.consensus.update_tip(H256::from_low_u64_be(1)); + let result = rx.await.unwrap(); assert_matches!( result, Ok(ExecOutput { done: false, reached_tip: false, stage_progress: 0 }) @@ -222,8 +223,9 @@ mod tests { runner.consensus.set_fail_validation(true); let input = ExecInput::default(); let seed = runner.seed_execution(input).expect("failed to seed execution"); - let result = runner.execute(input).await; + let rx = runner.execute(input); runner.after_execution(seed).await.expect("failed to run after execution hook"); + let result = rx.await.unwrap(); assert_matches!(result, Err(StageError::Validation { .. })); assert!(runner.validate_execution(input, result.ok()).is_ok(), "validation failed"); } @@ -238,7 +240,7 @@ mod tests { stage_progress: Some(stage_progress), }; let headers = runner.seed_execution(input).expect("failed to seed execution"); - let result = runner.execute(input).await; + let rx = runner.execute(input); // skip `after_execution` hook for linear downloader let tip = headers.last().unwrap(); @@ -253,6 +255,7 @@ mod tests { }) .await; + let result = rx.await.unwrap(); assert_matches!( result, Ok(ExecOutput { done: true, reached_tip: true, stage_progress }) if stage_progress == tip.number diff --git a/crates/stages/src/test_utils/macros.rs b/crates/stages/src/test_utils/macros.rs index 8ee33ba3cb0..f92c96c7dc1 100644 --- a/crates/stages/src/test_utils/macros.rs +++ b/crates/stages/src/test_utils/macros.rs @@ -10,7 +10,7 @@ macro_rules! stage_test_suite { let input = crate::stage::ExecInput::default(); // Run stage execution - let result = runner.execute(input).await; + let result = runner.execute(input).await.unwrap(); assert_matches!( result, Err(crate::error::StageError::DatabaseIntegrity(_)) @@ -34,12 +34,13 @@ macro_rules! stage_test_suite { let seed = runner.seed_execution(input).expect("failed to seed"); // Run stage execution - let result = runner.execute(input).await; + let rx = runner.execute(input); // Run `after_execution` hook runner.after_execution(seed).await.expect("failed to run after execution hook"); // Assert the successful result + let result = rx.await.unwrap(); assert_matches!( result, Ok(ExecOutput { done, reached_tip, stage_progress }) @@ -62,12 +63,13 @@ macro_rules! stage_test_suite { stage_progress: Some(stage_progress), }; let seed = runner.seed_execution(input).expect("failed to seed"); - let result = runner.execute(input).await; + let rx = runner.execute(input); // Run `after_execution` hook runner.after_execution(seed).await.expect("failed to run after execution hook"); // Assert the successful result + let result = rx.await.unwrap(); assert_matches!( result, Ok(ExecOutput { done, reached_tip, stage_progress }) @@ -110,10 +112,11 @@ macro_rules! stage_test_suite { let seed = runner.seed_execution(execute_input).expect("failed to seed"); // Run stage execution - let result = runner.execute(execute_input).await; + let rx = runner.execute(execute_input); runner.after_execution(seed).await.expect("failed to run after execution hook"); // Assert the successful execution result + let result = rx.await.unwrap(); assert_matches!( result, Ok(ExecOutput { done, reached_tip, stage_progress }) diff --git a/crates/stages/src/test_utils/runner.rs b/crates/stages/src/test_utils/runner.rs index 7b59c517fe0..761e9309599 100644 --- a/crates/stages/src/test_utils/runner.rs +++ b/crates/stages/src/test_utils/runner.rs @@ -41,7 +41,7 @@ pub(crate) trait ExecuteStageTestRunner: StageTestRunner { ) -> Result<(), TestRunnerError>; /// Run [Stage::execute] and return a receiver for the result. - async fn execute(&self, input: ExecInput) -> Result { + fn execute(&self, input: ExecInput) -> oneshot::Receiver> { let (tx, rx) = oneshot::channel(); let (db, mut stage) = (self.db().inner(), self.stage()); tokio::spawn(async move { @@ -50,7 +50,7 @@ pub(crate) trait ExecuteStageTestRunner: StageTestRunner { db.commit().expect("failed to commit"); tx.send(result).expect("failed to send message") }); - Box::pin(rx).await.unwrap() + rx } /// Run a hook after [Stage::execute]. Required for Headers & Bodies stages.