From 7d1c1fb84ee0f860b6f5be19be655b110c835488 Mon Sep 17 00:00:00 2001 From: teor Date: Sat, 19 Jun 2021 03:43:05 +1000 Subject: [PATCH] Document required request timeouts due to data dependencies (#2337) * Document required request timeouts due to data dependencies * Update AwaitUTXO docs --- zebra-consensus/src/chain.rs | 21 +++++++++++++++++++++ zebra-consensus/src/transaction.rs | 6 ++++++ zebra-state/src/lib.rs | 10 +++++++++- zebra-state/src/request.rs | 19 +++++++++++++++++-- 4 files changed, 53 insertions(+), 3 deletions(-) diff --git a/zebra-consensus/src/chain.rs b/zebra-consensus/src/chain.rs index e0b76356..7daa8c2f 100644 --- a/zebra-consensus/src/chain.rs +++ b/zebra-consensus/src/chain.rs @@ -2,6 +2,15 @@ //! //! Verifies blocks using the [`CheckpointVerifier`] or full [`BlockVerifier`], //! depending on the config and block height. +//! +//! # Correctness +//! +//! Block and transaction verification requests should be wrapped in a timeout, because: +//! - checkpoint verification waits for previous blocks, and +//! - full block and transaction verification wait for UTXOs from previous blocks. +//! +//! Otherwise, verification of out-of-order and invalid blocks and transactions can hang +//! indefinitely. #[cfg(test)] mod tests; @@ -47,6 +56,12 @@ const VERIFIER_BUFFER_BOUND: usize = 4; /// The chain verifier routes requests to either the checkpoint verifier or the /// block verifier, depending on the maximum checkpoint height. +/// +/// # Correctness +/// +/// Block verification requests should be wrapped in a timeout, so that +/// out-of-order and invalid requests do not hang indefinitely. See the [`chain`](`crate::chain`) +/// module documentation for details. struct ChainVerifier where S: Service + Send + Clone + 'static, @@ -132,6 +147,12 @@ where /// This function should only be called once for a particular state service. /// /// Dropped requests are cancelled on a best-effort basis, but may continue to be processed. +/// +/// # Correctness +/// +/// Block verification requests should be wrapped in a timeout, so that +/// out-of-order and invalid requests do not hang indefinitely. See the [`chain`](`crate::chain`) +/// module documentation for details. #[instrument(skip(state_service))] pub async fn init( config: Config, diff --git a/zebra-consensus/src/transaction.rs b/zebra-consensus/src/transaction.rs index fb1488a1..f8cd2e75 100644 --- a/zebra-consensus/src/transaction.rs +++ b/zebra-consensus/src/transaction.rs @@ -32,6 +32,12 @@ mod check; mod tests; /// Asynchronous transaction verification. +/// +/// # Correctness +/// +/// Transaction verification requests should be wrapped in a timeout, so that +/// out-of-order and invalid requests do not hang indefinitely. See the [`chain`](`crate::chain`) +/// module documentation for details. #[derive(Debug, Clone)] pub struct Verifier { network: Network, diff --git a/zebra-state/src/lib.rs b/zebra-state/src/lib.rs index 99c523e7..1c48a86c 100644 --- a/zebra-state/src/lib.rs +++ b/zebra-state/src/lib.rs @@ -1,4 +1,12 @@ -//! State storage code for Zebra. 🦓 +//! State contextual verification and storage code for Zebra. 🦓 +//! +//! # Correctness +//! +//! Await UTXO and block commit requests should be wrapped in a timeout, because: +//! - await UTXO requests wait for a block containing that UTXO, and +//! - contextual verification and state updates wait for all previous blocks. +//! +//! Otherwise, verification of out-of-order and invalid blocks can hang indefinitely. #![doc(html_favicon_url = "https://www.zfnd.org/images/zebra-favicon-128.png")] #![doc(html_logo_url = "https://www.zfnd.org/images/zebra-icon.png")] diff --git a/zebra-state/src/request.rs b/zebra-state/src/request.rs index 016d73fb..db70f199 100644 --- a/zebra-state/src/request.rs +++ b/zebra-state/src/request.rs @@ -186,6 +186,12 @@ pub enum Request { /// future will have no effect on whether it is eventually processed. A /// request to commit a block which has been queued internally but not yet /// committed will fail the older request and replace it with the newer request. + /// + /// # Correctness + /// + /// Block commit requests should be wrapped in a timeout, so that + /// out-of-order and invalid requests do not hang indefinitely. See the [`crate`] + /// documentation for details. CommitBlock(PreparedBlock), /// Commit a finalized block to the state, skipping all validation. @@ -202,6 +208,12 @@ pub enum Request { /// future will have no effect on whether it is eventually processed. /// Duplicate requests should not be made, because it is the caller's /// responsibility to ensure that each block is valid and final. + /// + /// # Correctness + /// + /// Block commit requests should be wrapped in a timeout, so that + /// out-of-order and invalid requests do not hang indefinitely. See the [`crate`] + /// documentation for details. CommitFinalizedBlock(FinalizedBlock), /// Computes the depth in the current best chain of the block identified by the given hash. @@ -252,8 +264,11 @@ pub enum Request { /// whether the UTXO remains unspent or is on the best chain, or any chain. /// Its purpose is to allow asynchronous script verification. /// - /// Code making this request should apply a timeout layer to the service to - /// handle missing UTXOs. + /// # Correctness + /// + /// UTXO requests should be wrapped in a timeout, so that + /// out-of-order and invalid requests do not hang indefinitely. See the [`crate`] + /// documentation for details. AwaitUtxo(transparent::OutPoint), /// Finds the first hash that's in the peer's `known_blocks` and the local best chain.