From f33923f12fbe3169eea4e1eaa0f2ad1a019751c3 Mon Sep 17 00:00:00 2001 From: Janito Vaqueiro Ferreira Filho Date: Wed, 30 Jun 2021 21:30:31 -0300 Subject: [PATCH] Create a shared Tokio runtime for tests (#2397) * Add a `zebra_test::RUNTIME` shared runtime Create a lazily instantiated Tokio runtime that can be shared by tests. * Split tests that require a shared runtime Split two tests that were previously in one because of the need to share a single Tokio runtime. With the `zebra_test::RUNTIME`, they can now share the runtime without having to be a single test. --- Cargo.lock | 1 + zebra-consensus/src/transaction/tests.rs | 135 +++++++++++++++-------- zebra-test/Cargo.toml | 2 + zebra-test/src/lib.rs | 23 ++++ 4 files changed, 115 insertions(+), 46 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4827d47b..1b73e843 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4655,6 +4655,7 @@ dependencies = [ "futures 0.3.15", "hex", "lazy_static", + "once_cell", "owo-colors 2.0.0", "pretty_assertions", "proptest", diff --git a/zebra-consensus/src/transaction/tests.rs b/zebra-consensus/src/transaction/tests.rs index fbc7eba2..651fb4ff 100644 --- a/zebra-consensus/src/transaction/tests.rs +++ b/zebra-consensus/src/transaction/tests.rs @@ -573,39 +573,30 @@ async fn v5_transaction_with_transparent_transfer_is_rejected_by_the_script() { ); } -/// Tests transactions with Sprout transfers. -/// -/// This is actually two tests: -/// - Test if signed V4 transaction with a dummy [`sprout::JoinSplit`] is accepted. +/// Test if signed V4 transaction with a dummy [`sprout::JoinSplit`] is accepted. /// - Test if an unsigned V4 transaction with a dummy [`sprout::JoinSplit`] is rejected. /// -/// The first test verifies if the transaction verifier correctly accepts a signed transaction. The -/// second test verifies if the transaction verifier correctly rejects the transaction because of -/// the invalid signature. -/// -/// These tests are grouped together because of a limitation to test shared [`tower_batch::Batch`] -/// services. Such services spawn a Tokio task in the runtime, and `#[tokio::test]` can create a -/// separate runtime for each test. This means that the worker task is created for one test and -/// destroyed before the other gets a chance to use it. (We'll fix this in #2390.) -#[tokio::test] -async fn v4_with_sprout_transfers() { - let network = Network::Mainnet; - let network_upgrade = NetworkUpgrade::Canopy; +/// This test verifies if the transaction verifier correctly accepts a signed transaction. +#[test] +fn v4_with_signed_sprout_transfer_is_accepted() { + zebra_test::init(); + zebra_test::RUNTIME.block_on(async { + let network = Network::Mainnet; + let network_upgrade = NetworkUpgrade::Canopy; - let canopy_activation_height = network_upgrade - .activation_height(network) - .expect("Canopy activation height is not set"); + let canopy_activation_height = network_upgrade + .activation_height(network) + .expect("Canopy activation height is not set"); - let transaction_block_height = - (canopy_activation_height + 10).expect("Canopy activation height is too large"); + let transaction_block_height = + (canopy_activation_height + 10).expect("Canopy activation height is too large"); - // Initialize the verifier - let state_service = - service_fn(|_| async { unreachable!("State service should not be called") }); - let script_verifier = script::Verifier::new(state_service); - let verifier = Verifier::new(network, script_verifier); + // Initialize the verifier + let state_service = + service_fn(|_| async { unreachable!("State service should not be called") }); + let script_verifier = script::Verifier::new(state_service); + let verifier = Verifier::new(network, script_verifier); - for should_sign in [true, false] { // Create a fake Sprout join split let (joinsplit_data, signing_key) = mock_sprout_join_split_data(); @@ -618,25 +609,67 @@ async fn v4_with_sprout_transfers() { sapling_shielded_data: None, }; - let expected_result = if should_sign { - // Sign the transaction - let sighash = transaction.sighash(network_upgrade, HashType::ALL, None); + // Sign the transaction + let sighash = transaction.sighash(network_upgrade, HashType::ALL, None); - match &mut transaction { - Transaction::V4 { - joinsplit_data: Some(joinsplit_data), - .. - } => joinsplit_data.sig = signing_key.sign(sighash.as_bytes()), - _ => unreachable!("Mock transaction was created incorrectly"), - } + match &mut transaction { + Transaction::V4 { + joinsplit_data: Some(joinsplit_data), + .. + } => joinsplit_data.sig = signing_key.sign(sighash.as_bytes()), + _ => unreachable!("Mock transaction was created incorrectly"), + } - Ok(transaction.hash()) - } else { - // TODO: Fix error downcast - // Err(TransactionError::Ed25519(ed25519::Error::InvalidSignature)) - Err(TransactionError::InternalDowncastError( - "downcast to redjubjub::Error failed, original error: InvalidSignature".to_string(), - )) + let expected_hash = transaction.hash(); + + // Test the transaction verifier + let result = verifier + .clone() + .oneshot(Request::Block { + transaction: Arc::new(transaction), + known_utxos: Arc::new(HashMap::new()), + height: transaction_block_height, + }) + .await; + + assert_eq!(result, Ok(expected_hash)); + }); +} + +/// Test if an unsigned V4 transaction with a dummy [`sprout::JoinSplit`] is rejected. +/// +/// This test verifies if the transaction verifier correctly rejects the transaction because of the +/// invalid signature. +#[test] +fn v4_with_unsigned_sprout_transfer_is_rejected() { + zebra_test::init(); + zebra_test::RUNTIME.block_on(async { + let network = Network::Mainnet; + let network_upgrade = NetworkUpgrade::Canopy; + + let canopy_activation_height = network_upgrade + .activation_height(network) + .expect("Canopy activation height is not set"); + + let transaction_block_height = + (canopy_activation_height + 10).expect("Canopy activation height is too large"); + + // Initialize the verifier + let state_service = + service_fn(|_| async { unreachable!("State service should not be called") }); + let script_verifier = script::Verifier::new(state_service); + let verifier = Verifier::new(network, script_verifier); + + // Create a fake Sprout join split + let (joinsplit_data, _) = mock_sprout_join_split_data(); + + let transaction = Transaction::V4 { + inputs: vec![], + outputs: vec![], + lock_time: LockTime::Height(block::Height(0)), + expiry_height: (transaction_block_height + 1).expect("expiry height is too large"), + joinsplit_data: Some(joinsplit_data), + sapling_shielded_data: None, }; // Test the transaction verifier @@ -649,8 +682,18 @@ async fn v4_with_sprout_transfers() { }) .await; - assert_eq!(result, expected_result); - } + assert_eq!( + result, + Err( + // TODO: Fix error downcast + // Err(TransactionError::Ed25519(ed25519::Error::InvalidSignature)) + TransactionError::InternalDowncastError( + "downcast to redjubjub::Error failed, original error: InvalidSignature" + .to_string(), + ) + ) + ); + }); } // Utility functions diff --git a/zebra-test/Cargo.toml b/zebra-test/Cargo.toml index eaf59ac2..b60a7531 100644 --- a/zebra-test/Cargo.toml +++ b/zebra-test/Cargo.toml @@ -10,11 +10,13 @@ edition = "2018" [dependencies] hex = "0.4.3" lazy_static = "1.4.0" +once_cell = "1.8" proptest = "0.10.1" rand = "0.8" regex = "1.4.6" tower = { version = "0.4", features = ["util"] } +tokio = { version = "0.3", features = ["rt-multi-thread"] } futures = "0.3.15" color-eyre = "0.5.11" diff --git a/zebra-test/src/lib.rs b/zebra-test/src/lib.rs index b36ceef5..389affe0 100644 --- a/zebra-test/src/lib.rs +++ b/zebra-test/src/lib.rs @@ -11,6 +11,7 @@ #![recursion_limit = "256"] use color_eyre::section::PanicMessage; +use once_cell::sync::Lazy; use owo_colors::OwoColorize; use tracing_error::ErrorLayer; use tracing_subscriber::{fmt, prelude::*, EnvFilter}; @@ -24,6 +25,28 @@ pub mod prelude; pub mod transcript; pub mod vectors; +/// A multi-threaded Tokio runtime that can be shared between tests. +/// +/// This shared runtime should be used in tests that use shared background tasks. An example is +/// with shared global `Lazy` types, because they spawn a background task when they +/// are first initialized. This background task is stopped when the runtime is shut down, so having +/// a runtime per test means that only the first test actually manages to successfully use the +/// background task. Using the shared runtime allows the background task to keep running for the +/// other tests that also use it. +/// +/// A shared runtime should not be used in tests that need to pause and resume the Tokio timer. +/// This is because multiple tests might be sharing the runtime at the same time, so there could be +/// conflicts with pausing and resuming the timer at incorrect points. Even if only one test runs +/// at a time, there's a risk of a test finishing while the timer is paused (due to a test failure, +/// for example) and that means that the next test will already start with an incorrect timer +/// state. +pub static RUNTIME: Lazy = Lazy::new(|| { + tokio::runtime::Builder::new_multi_thread() + .enable_all() + .build() + .expect("Failed to create Tokio runtime") +}); + static INIT: Once = Once::new(); /// Initialize globals for tests such as the tracing subscriber and panic / error