From 39dfca8e64cc3fbb6518d578b7f6707f5ef6f6c6 Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 18 Mar 2022 08:59:46 +1000 Subject: [PATCH] 5. change(state): split ReadStateService requests into a ReadRequest enum (#3866) * Split out ReadRequest and ReadResponse state service enums * Simplify RPC test vectors * Split state requests into Request and ReadRequest * Make zebra-rpc use the new state ReadRequest --- zebra-rpc/src/methods.rs | 19 +-- zebra-rpc/src/methods/tests/vectors.rs | 23 ++-- zebra-rpc/src/server.rs | 4 +- zebra-state/src/lib.rs | 6 +- zebra-state/src/request.rs | 27 ++++- zebra-state/src/response.rs | 15 ++- zebra-state/src/service.rs | 109 ++++++++---------- zebra-state/src/service/read.rs | 3 + zebra-state/src/service/read/tests.rs | 3 + zebra-state/src/service/read/tests/vectors.rs | 102 ++++++++++++++++ zebra-state/src/service/tests.rs | 5 + zebra-state/tests/basic.rs | 6 +- zebra-test/src/transcript.rs | 3 + 13 files changed, 233 insertions(+), 92 deletions(-) create mode 100644 zebra-state/src/service/read/tests.rs create mode 100644 zebra-state/src/service/read/tests/vectors.rs diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs index c5bc253c10b..0a2259e3bab 100644 --- a/zebra-rpc/src/methods.rs +++ b/zebra-rpc/src/methods.rs @@ -110,8 +110,8 @@ pub struct RpcImpl where Mempool: Service, State: Service< - zebra_state::Request, - Response = zebra_state::Response, + zebra_state::ReadRequest, + Response = zebra_state::ReadResponse, Error = zebra_state::BoxError, >, Tip: ChainTip, @@ -137,8 +137,8 @@ impl RpcImpl where Mempool: Service, State: Service< - zebra_state::Request, - Response = zebra_state::Response, + zebra_state::ReadRequest, + Response = zebra_state::ReadResponse, Error = zebra_state::BoxError, >, Tip: ChainTip + Send + Sync, @@ -170,8 +170,8 @@ where tower::Service + 'static, Mempool::Future: Send, State: Service< - zebra_state::Request, - Response = zebra_state::Response, + zebra_state::ReadRequest, + Response = zebra_state::ReadResponse, Error = zebra_state::BoxError, > + Clone + Send @@ -257,7 +257,8 @@ where data: None, })?; - let request = zebra_state::Request::Block(zebra_state::HashOrHeight::Height(height)); + let request = + zebra_state::ReadRequest::Block(zebra_state::HashOrHeight::Height(height)); let response = state .ready() .and_then(|service| service.call(request)) @@ -269,8 +270,8 @@ where })?; match response { - zebra_state::Response::Block(Some(block)) => Ok(GetBlock(block.into())), - zebra_state::Response::Block(None) => Err(Error { + zebra_state::ReadResponse::Block(Some(block)) => Ok(GetBlock(block.into())), + zebra_state::ReadResponse::Block(None) => Err(Error { code: ErrorCode::ServerError(0), message: "Block not found".to_string(), data: None, diff --git a/zebra-rpc/src/methods/tests/vectors.rs b/zebra-rpc/src/methods/tests/vectors.rs index 17df187228f..36dcb8a9cf3 100644 --- a/zebra-rpc/src/methods/tests/vectors.rs +++ b/zebra-rpc/src/methods/tests/vectors.rs @@ -15,9 +15,6 @@ use zebra_test::mock_service::MockService; use super::super::*; -// Number of blocks to populate state with -const NUMBER_OF_BLOCKS: u32 = 10; - #[tokio::test] async fn rpc_getinfo() { zebra_test::init(); @@ -51,10 +48,10 @@ async fn rpc_getinfo() { async fn rpc_getblock() { zebra_test::init(); - // Put the first `NUMBER_OF_BLOCKS` blocks in a vector - let blocks: Vec> = zebra_test::vectors::MAINNET_BLOCKS - .range(0..=NUMBER_OF_BLOCKS) - .map(|(_, block_bytes)| block_bytes.zcash_deserialize_into::>().unwrap()) + // Create a continuous chain of mainnet blocks from genesis + let blocks: Vec> = zebra_test::vectors::CONTINUOUS_MAINNET_BLOCKS + .iter() + .map(|(_height, block_bytes)| block_bytes.zcash_deserialize_into().unwrap()) .collect(); let mut mempool: MockService<_, _, _, BoxError> = MockService::build().for_unit_tests(); @@ -114,17 +111,15 @@ async fn rpc_getblock_error() { async fn rpc_getbestblockhash() { zebra_test::init(); - // Put `NUMBER_OF_BLOCKS` blocks in a vector - let blocks: Vec> = zebra_test::vectors::MAINNET_BLOCKS - .range(0..=NUMBER_OF_BLOCKS) - .map(|(_, block_bytes)| block_bytes.zcash_deserialize_into::>().unwrap()) + // Create a continuous chain of mainnet blocks from genesis + let blocks: Vec> = zebra_test::vectors::CONTINUOUS_MAINNET_BLOCKS + .iter() + .map(|(_height, block_bytes)| block_bytes.zcash_deserialize_into().unwrap()) .collect(); // Get the hash of the block at the tip using hardcoded block tip bytes. // We want to test the RPC response is equal to this hash - let tip_block: Block = zebra_test::vectors::BLOCK_MAINNET_10_BYTES - .zcash_deserialize_into() - .unwrap(); + let tip_block = blocks.last().unwrap(); let tip_block_hash = tip_block.hash(); // Get a mempool handle diff --git a/zebra-rpc/src/server.rs b/zebra-rpc/src/server.rs index c97b42f3dea..523924cc5d4 100644 --- a/zebra-rpc/src/server.rs +++ b/zebra-rpc/src/server.rs @@ -44,8 +44,8 @@ impl RpcServer { + 'static, Mempool::Future: Send, State: Service< - zebra_state::Request, - Response = zebra_state::Response, + zebra_state::ReadRequest, + Response = zebra_state::ReadResponse, Error = zebra_state::BoxError, > + Clone + Send diff --git a/zebra-state/src/lib.rs b/zebra-state/src/lib.rs index de01ed442d4..454ca9bb81a 100644 --- a/zebra-state/src/lib.rs +++ b/zebra-state/src/lib.rs @@ -31,8 +31,8 @@ mod tests; pub use config::Config; pub use constants::MAX_BLOCK_REORG_HEIGHT; pub use error::{BoxError, CloneError, CommitBlockError, ValidateContextError}; -pub use request::{FinalizedBlock, HashOrHeight, PreparedBlock, Request}; -pub use response::Response; +pub use request::{FinalizedBlock, HashOrHeight, PreparedBlock, ReadRequest, Request}; +pub use response::{ReadResponse, Response}; pub use service::{ chain_tip::{ChainTipChange, LatestChainTip, TipAction}, init, @@ -42,7 +42,7 @@ pub use service::{ pub use service::{ arbitrary::populated_state, chain_tip::{ChainTipBlock, ChainTipSender}, - init_test, + init_test, init_test_services, }; pub(crate) use request::ContextuallyValidBlock; diff --git a/zebra-state/src/request.rs b/zebra-state/src/request.rs index 5f886290c01..985f6932e43 100644 --- a/zebra-state/src/request.rs +++ b/zebra-state/src/request.rs @@ -1,3 +1,5 @@ +//! State [`tower::Service`] request types. + use std::{collections::HashMap, sync::Arc}; use zebra_chain::{ @@ -229,7 +231,7 @@ impl From for FinalizedBlock { } #[derive(Clone, Debug, PartialEq, Eq)] -/// A query about or modification to the chain state. +/// A query about or modification to the chain state, via the [`StateService`]. pub enum Request { /// Performs contextual validation of the given block, committing it to the /// state if successful. @@ -377,3 +379,26 @@ pub enum Request { stop: Option, }, } + +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +/// A read-only query about the chain state, via the [`ReadStateService`]. +pub enum ReadRequest { + /// Looks up a block by hash or height in the current best chain. + /// + /// Returns + /// + /// * [`Response::Block(Some(Arc))`](Response::Block) if the block is in the best chain; + /// * [`Response::Block(None)`](Response::Block) otherwise. + /// + /// Note: the [`HashOrHeight`] can be constructed from a [`block::Hash`] or + /// [`block::Height`] using `.into()`. + Block(HashOrHeight), + + /// Looks up a transaction by hash in the current best chain. + /// + /// Returns + /// + /// * [`Response::Transaction(Some(Arc))`](Response::Transaction) if the transaction is in the best chain; + /// * [`Response::Transaction(None)`](Response::Transaction) otherwise. + Transaction(transaction::Hash), +} diff --git a/zebra-state/src/response.rs b/zebra-state/src/response.rs index efadb3b89b2..8b49019a89b 100644 --- a/zebra-state/src/response.rs +++ b/zebra-state/src/response.rs @@ -1,4 +1,7 @@ +//! State [`tower::Service`] response types. + use std::sync::Arc; + use zebra_chain::{ block::{self, Block}, transaction::Transaction, @@ -11,7 +14,7 @@ use zebra_chain::{ use crate::Request; #[derive(Clone, Debug, PartialEq, Eq)] -/// A response to a state [`Request`]. +/// A response to a [`StateService`] [`Request`]. pub enum Response { /// Response to [`Request::CommitBlock`] indicating that a block was /// successfully committed to the state. @@ -41,3 +44,13 @@ pub enum Response { /// The response to a `FindBlockHeaders` request. BlockHeaders(Vec), } + +#[derive(Clone, Debug, PartialEq, Eq)] +/// A response to a read-only [`ReadStateService`] [`ReadRequest`]. +pub enum ReadResponse { + /// Response to [`ReadRequest::Block`] with the specified block. + Block(Option>), + + /// Response to [`ReadRequest::Transaction`] with the specified transaction. + Transaction(Option>), +} diff --git a/zebra-state/src/service.rs b/zebra-state/src/service.rs index 9deb3c4ed15..7c4b553e3e5 100644 --- a/zebra-state/src/service.rs +++ b/zebra-state/src/service.rs @@ -44,8 +44,8 @@ use crate::{ pending_utxos::PendingUtxos, watch_receiver::WatchReceiver, }, - BoxError, CloneError, CommitBlockError, Config, FinalizedBlock, PreparedBlock, Request, - Response, ValidateContextError, + BoxError, CloneError, CommitBlockError, Config, FinalizedBlock, PreparedBlock, ReadRequest, + ReadResponse, Request, Response, ValidateContextError, }; pub mod block_iter; @@ -902,8 +902,8 @@ impl Service for StateService { } } -impl Service for ReadStateService { - type Response = Response; +impl Service for ReadStateService { + type Response = ReadResponse; type Error = BoxError; type Future = Pin> + Send + 'static>>; @@ -913,10 +913,19 @@ impl Service for ReadStateService { } #[instrument(name = "read_state", skip(self))] - fn call(&mut self, req: Request) -> Self::Future { + fn call(&mut self, req: ReadRequest) -> Self::Future { match req { + // TODO: implement these new ReadRequests for lightwalletd, as part of these tickets + + // z_get_tree_state (#3156) + + // depends on transparent address indexes (#3150) + // get_address_tx_ids (#3147) + // get_address_balance (#3157) + // get_address_utxos (#3158) + // Used by get_block RPC. - Request::Block(hash_or_height) => { + ReadRequest::Block(hash_or_height) => { metrics::counter!( "state.requests", 1, @@ -931,13 +940,13 @@ impl Service for ReadStateService { read::block(best_chain, &state.db, hash_or_height) }); - Ok(Response::Block(block)) + Ok(ReadResponse::Block(block)) } .boxed() } // For the get_raw_transaction RPC, to be implemented in #3145. - Request::Transaction(hash) => { + ReadRequest::Transaction(hash) => { metrics::counter!( "state.requests", 1, @@ -952,60 +961,10 @@ impl Service for ReadStateService { read::transaction(best_chain, &state.db, hash) }); - Ok(Response::Transaction(transaction)) + Ok(ReadResponse::Transaction(transaction)) } .boxed() } - - // TODO: split the Request enum, then implement these new ReadRequests for lightwalletd - // as part of these tickets - - // z_get_tree_state (#3156) - - // depends on transparent address indexes (#3150) - // get_address_tx_ids (#3147) - // get_address_balance (#3157) - // get_address_utxos (#3158) - - // Out of Scope - // TODO: delete when splitting the Request enum - - // Use ChainTip instead. - Request::Tip => unreachable!("ReadStateService doesn't need to Tip"), - - // These requests don't need better performance at the moment. - Request::FindBlockHashes { - known_blocks: _, - stop: _, - } => { - unreachable!("ReadStateService doesn't need to FindBlockHashes") - } - Request::FindBlockHeaders { - known_blocks: _, - stop: _, - } => { - unreachable!("ReadStateService doesn't need to FindBlockHeaders") - } - - // Some callers of this request need to wait for queued blocks. - Request::Depth(_hash) => unreachable!("ReadStateService could change depth behaviour"), - - // This request needs to wait for queued blocks. - Request::BlockLocator => { - unreachable!("ReadStateService should not be used for block locators") - } - - // Impossible Requests - - // The read-only service doesn't have the shared internal state - // needed to await UTXOs. - Request::AwaitUtxo(_outpoint) => unreachable!("ReadStateService can't await UTXOs"), - - // The read-only service can't write. - Request::CommitBlock(_prepared) => unreachable!("ReadStateService can't commit blocks"), - Request::CommitFinalizedBlock(_finalized) => { - unreachable!("ReadStateService can't commit blocks") - } } } } @@ -1042,12 +1001,40 @@ pub fn init( ) } -/// Initialize a state service with an ephemeral [`Config`] and a buffer with a single slot. +/// Returns a [`StateService`] with an ephemeral [`Config`] and a buffer with a single slot. /// -/// This can be used to create a state service for testing. See also [`init`]. +/// This can be used to create a state service for testing. +/// +/// See also [`init`]. #[cfg(any(test, feature = "proptest-impl"))] pub fn init_test(network: Network) -> Buffer, Request> { let (state_service, _, _, _) = StateService::new(Config::ephemeral(), network); Buffer::new(BoxService::new(state_service), 1) } + +/// Initializes a state service with an ephemeral [`Config`] and a buffer with a single slot, +/// then returns the read-write service, read-only service, and tip watch channels. +/// +/// This can be used to create a state service for testing. See also [`init`]. +#[cfg(any(test, feature = "proptest-impl"))] +pub fn init_test_services( + network: Network, +) -> ( + Buffer, Request>, + ReadStateService, + LatestChainTip, + ChainTipChange, +) { + let (state_service, read_state_service, latest_chain_tip, chain_tip_change) = + StateService::new(Config::ephemeral(), network); + + let state_service = Buffer::new(BoxService::new(state_service), 1); + + ( + state_service, + read_state_service, + latest_chain_tip, + chain_tip_change, + ) +} diff --git a/zebra-state/src/service/read.rs b/zebra-state/src/service/read.rs index 4d4d924769f..b76989384b2 100644 --- a/zebra-state/src/service/read.rs +++ b/zebra-state/src/service/read.rs @@ -16,6 +16,9 @@ use crate::{ HashOrHeight, }; +#[cfg(test)] +mod tests; + /// Returns the [`Block`] with [`block::Hash`](zebra_chain::block::Hash) or /// [`Height`](zebra_chain::block::Height), /// if it exists in the non-finalized `chain` or finalized `db`. diff --git a/zebra-state/src/service/read/tests.rs b/zebra-state/src/service/read/tests.rs new file mode 100644 index 00000000000..1905ef32b5a --- /dev/null +++ b/zebra-state/src/service/read/tests.rs @@ -0,0 +1,3 @@ +//! Tests for the ReadStateService. + +mod vectors; diff --git a/zebra-state/src/service/read/tests/vectors.rs b/zebra-state/src/service/read/tests/vectors.rs new file mode 100644 index 00000000000..db133686ead --- /dev/null +++ b/zebra-state/src/service/read/tests/vectors.rs @@ -0,0 +1,102 @@ +//! Fixed test vectors for the ReadStateService. + +use std::sync::Arc; + +use zebra_chain::{ + block::Block, parameters::Network::*, serialization::ZcashDeserializeInto, transaction, +}; + +use zebra_test::{ + prelude::Result, + transcript::{ExpectedTranscriptError, Transcript}, +}; + +use crate::{init_test_services, populated_state, ReadRequest, ReadResponse}; + +/// Test that ReadStateService responds correctly when empty. +#[tokio::test] +async fn empty_read_state_still_responds_to_requests() -> Result<()> { + zebra_test::init(); + + let transcript = Transcript::from(empty_state_test_cases()); + + let network = Mainnet; + let (_state, read_state, _latest_chain_tip, _chain_tip_change) = init_test_services(network); + + transcript.check(read_state).await?; + + Ok(()) +} + +/// Test that ReadStateService responds correctly when the state contains blocks. +#[tokio::test] +async fn populated_read_state_responds_correctly() -> Result<()> { + zebra_test::init(); + + // Create a continuous chain of mainnet blocks from genesis + let blocks: Vec> = zebra_test::vectors::CONTINUOUS_MAINNET_BLOCKS + .iter() + .map(|(_height, block_bytes)| block_bytes.zcash_deserialize_into().unwrap()) + .collect(); + + let (_state, read_state, _latest_chain_tip, _chain_tip_change) = + populated_state(blocks.clone(), Mainnet).await; + + let empty_cases = Transcript::from(empty_state_test_cases()); + empty_cases.check(read_state.clone()).await?; + + for block in blocks { + let block_cases = vec![ + ( + ReadRequest::Block(block.hash().into()), + Ok(ReadResponse::Block(Some(block.clone()))), + ), + ( + ReadRequest::Block(block.coinbase_height().unwrap().into()), + Ok(ReadResponse::Block(Some(block.clone()))), + ), + ]; + + let block_cases = Transcript::from(block_cases); + block_cases.check(read_state.clone()).await?; + + // Spec: transactions in the genesis block are ignored. + if block.coinbase_height().unwrap().0 == 0 { + continue; + } + + for transaction in &block.transactions { + let transaction_cases = vec![( + ReadRequest::Transaction(transaction.hash()), + Ok(ReadResponse::Transaction(Some(transaction.clone()))), + )]; + + let transaction_cases = Transcript::from(transaction_cases); + transaction_cases.check(read_state.clone()).await?; + } + } + + Ok(()) +} + +/// Returns test cases for the empty state and missing blocks. +fn empty_state_test_cases() -> Vec<(ReadRequest, Result)> { + let block: Arc = zebra_test::vectors::BLOCK_MAINNET_419200_BYTES + .zcash_deserialize_into() + .unwrap(); + + vec![ + ( + ReadRequest::Transaction(transaction::Hash([0; 32])), + Ok(ReadResponse::Transaction(None)), + ), + ( + ReadRequest::Block(block.hash().into()), + Ok(ReadResponse::Block(None)), + ), + ( + ReadRequest::Block(block.coinbase_height().unwrap().into()), + Ok(ReadResponse::Block(None)), + ), + ] +} diff --git a/zebra-state/src/service/tests.rs b/zebra-state/src/service/tests.rs index 1893cd1f2e8..7c9955053af 100644 --- a/zebra-state/src/service/tests.rs +++ b/zebra-state/src/service/tests.rs @@ -1,3 +1,7 @@ +//! StateService test vectors. +//! +//! TODO: move these tests into tests::vectors and tests::prop modules. + use std::{convert::TryInto, env, sync::Arc}; use tower::{buffer::Buffer, util::BoxService}; @@ -11,6 +15,7 @@ use zebra_chain::{ transaction, transparent, value_balance::ValueBalance, }; + use zebra_test::{prelude::*, transcript::Transcript}; use crate::{ diff --git a/zebra-state/tests/basic.rs b/zebra-state/tests/basic.rs index 4f3714dbbce..b2b07fa4c77 100644 --- a/zebra-state/tests/basic.rs +++ b/zebra-state/tests/basic.rs @@ -1,6 +1,10 @@ +//! Basic integration tests for zebra-state + +use std::sync::Arc; + use color_eyre::eyre::Report; use once_cell::sync::Lazy; -use std::sync::Arc; + use zebra_chain::{block::Block, parameters::Network, serialization::ZcashDeserialize}; use zebra_test::transcript::{ExpectedTranscriptError, Transcript}; diff --git a/zebra-test/src/transcript.rs b/zebra-test/src/transcript.rs index 04ecd7e12df..aeac6705e63 100644 --- a/zebra-test/src/transcript.rs +++ b/zebra-test/src/transcript.rs @@ -37,6 +37,7 @@ impl ExpectedTranscriptError { } /// Check the actual error `e` against this expected error. + #[track_caller] fn check(&self, e: BoxError) -> Result<(), Report> { match self { ExpectedTranscriptError::Any => Ok(()), @@ -89,6 +90,7 @@ where S: Debug + Eq, { /// Check this transcript against the responses from the `to_check` service + #[track_caller] pub async fn check(mut self, mut to_check: C) -> Result<(), Report> where C: Service, @@ -169,6 +171,7 @@ where Poll::Ready(Ok(())) } + #[track_caller] fn call(&mut self, request: R) -> Self::Future { if let Some((expected_request, response)) = self.messages.next() { match response {