diff --git a/zebra-consensus/src/transaction/tests.rs b/zebra-consensus/src/transaction/tests.rs index 9f9fd6fbf..90608537b 100644 --- a/zebra-consensus/src/transaction/tests.rs +++ b/zebra-consensus/src/transaction/tests.rs @@ -2016,30 +2016,52 @@ async fn v4_with_joinsplit_is_rejected_for_modification( .rev() .filter(|(_, transaction)| !transaction.is_coinbase() && transaction.inputs().is_empty()) .find(|(_, transaction)| transaction.sprout_groth16_joinsplits().next().is_some()) - .expect("No transaction found with Groth16 JoinSplits"); + .expect("There should be a tx with Groth16 JoinSplits."); - modify_joinsplit( - Arc::get_mut(&mut transaction).expect("Transaction only has one active reference"), - modification, - ); + let expected_error = Err(expected_error); + + // Modify a JoinSplit in the transaction following the given modification type. + let tx = Arc::get_mut(&mut transaction).expect("The tx should have only one active reference."); + match tx { + Transaction::V4 { + joinsplit_data: Some(ref mut joinsplit_data), + .. + } => modify_joinsplit_data(joinsplit_data, modification), + _ => unreachable!("Transaction should have some JoinSplit shielded data."), + } // Initialize the verifier let state_service = - service_fn(|_| async { unreachable!("State service should not be called") }); + service_fn(|_| async { unreachable!("State service should not be called.") }); let verifier = Verifier::new(network, state_service); - // Test the transaction verifier - let result = verifier - .clone() - .oneshot(Request::Block { - transaction, - known_utxos: Arc::new(HashMap::new()), - height, - time: DateTime::::MAX_UTC, - }) - .await; + // Test the transaction verifier. + // + // Note that modifying the JoinSplit data invalidates the tx signatures. The signatures are + // checked concurrently with the ZK proofs, and when a signature check finishes before the proof + // check, the verifier reports an invalid signature instead of invalid proof. This race + // condition happens only occasionaly, so we run the verifier in a loop with a small iteration + // threshold until it returns the correct error. + let mut i = 1; + let result = loop { + let result = verifier + .clone() + .oneshot(Request::Block { + transaction: transaction.clone(), + known_utxos: Arc::new(HashMap::new()), + height, + time: DateTime::::MAX_UTC, + }) + .await; - assert_eq!(result, Err(expected_error)); + if result == expected_error || i >= 10 { + break result; + } + + i += 1; + }; + + assert_eq!(result, expected_error); } /// Test if a V4 transaction with Sapling spends is accepted by the verifier. @@ -2496,17 +2518,6 @@ enum JoinSplitModification { ZeroProof, } -/// Modify a JoinSplit in the transaction following the given modification type. -fn modify_joinsplit(transaction: &mut Transaction, modification: JoinSplitModification) { - match transaction { - Transaction::V4 { - joinsplit_data: Some(ref mut joinsplit_data), - .. - } => modify_joinsplit_data(joinsplit_data, modification), - _ => unreachable!("Transaction has no JoinSplit shielded data"), - } -} - /// Modify a [`JoinSplitData`] following the given modification type. fn modify_joinsplit_data( joinsplit_data: &mut JoinSplitData,