From 3d2c5ef29097f25558741e145772e805cfbe668f Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 22 Jun 2023 06:44:53 +1000 Subject: [PATCH] fix(concurrency): Use Arc::into_inner() to avoid potential concurrency issues, needs Rust 1.70 (#7032) * Use Arc::into_inner() to avoid potential concurrency issues * Remove some outdated clippy lint workarounds (fixed in Rust 1.66) * Update the required Rust version to 1.70 --- zebra-chain/src/orchard/shielded_data.rs | 3 --- zebra-chain/src/transaction/serialize.rs | 3 --- zebra-consensus/src/block.rs | 2 +- zebra-network/src/protocol/external/codec.rs | 2 -- zebra-state/src/service.rs | 12 ++++-------- zebra-state/src/service/non_finalized_state.rs | 2 ++ zebra-utils/Cargo.toml | 4 ++++ zebrad/Cargo.toml | 2 +- 8 files changed, 12 insertions(+), 18 deletions(-) diff --git a/zebra-chain/src/orchard/shielded_data.rs b/zebra-chain/src/orchard/shielded_data.rs index dc55d19a8..3a034c05f 100644 --- a/zebra-chain/src/orchard/shielded_data.rs +++ b/zebra-chain/src/orchard/shielded_data.rs @@ -269,9 +269,6 @@ impl ZcashDeserialize for Flags { // Consensus rule: "In a version 5 transaction, // the reserved bits 2..7 of the flagsOrchard field MUST be zero." // https://zips.z.cash/protocol/protocol.pdf#txnencodingandconsensus - // - // Clippy 1.64 is wrong here, this lazy evaluation is necessary, constructors are functions. This is fixed in 1.66. - #[allow(clippy::unnecessary_lazy_evaluations)] Flags::from_bits(reader.read_u8()?) .ok_or_else(|| SerializationError::Parse("invalid reserved orchard flags")) } diff --git a/zebra-chain/src/transaction/serialize.rs b/zebra-chain/src/transaction/serialize.rs index da6a3770b..f79244da6 100644 --- a/zebra-chain/src/transaction/serialize.rs +++ b/zebra-chain/src/transaction/serialize.rs @@ -884,9 +884,6 @@ impl ZcashDeserialize for Transaction { } // Denoted as `nConsensusBranchId` in the spec. // Convert it to a NetworkUpgrade - // - // Clippy 1.64 is wrong here, this lazy evaluation is necessary, constructors are functions. This is fixed in 1.66. - #[allow(clippy::unnecessary_lazy_evaluations)] let network_upgrade = NetworkUpgrade::from_branch_id(limited_reader.read_u32::()?) .ok_or_else(|| { diff --git a/zebra-consensus/src/block.rs b/zebra-consensus/src/block.rs index 3b694ac67..970cf4118 100644 --- a/zebra-consensus/src/block.rs +++ b/zebra-consensus/src/block.rs @@ -280,7 +280,7 @@ where check::miner_fees_are_valid(&block, network, block_miner_fees)?; // Finally, submit the block for contextual verification. - let new_outputs = Arc::try_unwrap(known_utxos) + let new_outputs = Arc::into_inner(known_utxos) .expect("all verification tasks using known_utxos are complete"); let prepared_block = zs::SemanticallyVerifiedBlock { diff --git a/zebra-network/src/protocol/external/codec.rs b/zebra-network/src/protocol/external/codec.rs index 7aee299da..6a4ae0585 100644 --- a/zebra-network/src/protocol/external/codec.rs +++ b/zebra-network/src/protocol/external/codec.rs @@ -500,8 +500,6 @@ impl Codec { /// Note: zcashd only requires fields up to `address_recv`, but everything up to `relay` is required in Zebra. /// see fn read_version(&self, mut reader: R) -> Result { - // Clippy 1.64 is wrong here, this lazy evaluation is necessary, constructors are functions. This is fixed in 1.66. - #[allow(clippy::unnecessary_lazy_evaluations)] Ok(VersionMessage { version: Version(reader.read_u32::()?), // Use from_bits_truncate to discard unknown service bits. diff --git a/zebra-state/src/service.rs b/zebra-state/src/service.rs index f88cd2811..0e7c96d17 100644 --- a/zebra-state/src/service.rs +++ b/zebra-state/src/service.rs @@ -269,7 +269,7 @@ impl Drop for ReadStateService { // so dropping it should check if we can shut down. if let Some(block_write_task) = self.block_write_task.take() { - if let Ok(block_write_task_handle) = Arc::try_unwrap(block_write_task) { + if let Some(block_write_task_handle) = Arc::into_inner(block_write_task) { // We're the last database user, so we can tell it to shut down (blocking): // - flushes the database to disk, and // - drops the database, which cleans up any database tasks correctly. @@ -1165,15 +1165,11 @@ impl Service for ReadStateService { if let Some(block_write_task) = block_write_task { if block_write_task.is_finished() { - match Arc::try_unwrap(block_write_task) { + if let Some(block_write_task) = Arc::into_inner(block_write_task) { // We are the last state with a reference to this task, so we can propagate any panics - Ok(block_write_task_handle) => { - if let Err(thread_panic) = block_write_task_handle.join() { - std::panic::resume_unwind(thread_panic); - } + if let Err(thread_panic) = block_write_task.join() { + std::panic::resume_unwind(thread_panic); } - // We're not the last state, so we need to put it back - Err(arc_block_write_task) => self.block_write_task = Some(arc_block_write_task), } } else { // It hasn't finished, so we need to put it back diff --git a/zebra-state/src/service/non_finalized_state.rs b/zebra-state/src/service/non_finalized_state.rs index bdcb1c10e..6cb9a2d44 100644 --- a/zebra-state/src/service/non_finalized_state.rs +++ b/zebra-state/src/service/non_finalized_state.rs @@ -399,6 +399,8 @@ impl NonFinalizedState { // Pushing a block onto a Chain can launch additional parallel batches. // TODO: should we pass _scope into Chain::push()? scope.spawn_fifo(|_scope| { + // TODO: Replace with Arc::unwrap_or_clone() when it stabilises: + // https://github.com/rust-lang/rust/issues/93610 let new_chain = Arc::try_unwrap(new_chain) .unwrap_or_else(|shared_chain| (*shared_chain).clone()); chain_push_result = Some(new_chain.push(contextual).map(Arc::new)); diff --git a/zebra-utils/Cargo.toml b/zebra-utils/Cargo.toml index 98e3b042c..9ce5fe076 100644 --- a/zebra-utils/Cargo.toml +++ b/zebra-utils/Cargo.toml @@ -15,6 +15,10 @@ keywords = ["zebra", "zcash"] # Must be one of categories = ["command-line-utilities", "cryptography::cryptocurrencies"] +# Zebra is only supported on the latest stable Rust version. See the README for details. +# Any Zebra release can break compatibility with older Rust versions. +rust-version = "1.70" + [[bin]] name = "zebra-checkpoints" # this setting is required for Zebra's Docker build caches diff --git a/zebrad/Cargo.toml b/zebrad/Cargo.toml index cfeef6df2..3ec95f01b 100644 --- a/zebrad/Cargo.toml +++ b/zebrad/Cargo.toml @@ -19,7 +19,7 @@ edition = "2021" # Zebra is only supported on the latest stable Rust version. See the README for details. # Any Zebra release can break compatibility with older Rust versions. -rust-version = "1.66" +rust-version = "1.70" # Settings that impact runtime behaviour