From 1e5768d703ca7d720144d6d5b72a591096851193 Mon Sep 17 00:00:00 2001 From: Alexander Cyon Date: Thu, 29 Aug 2024 08:41:17 +0200 Subject: [PATCH] fix bug and added more tests. --- .../signing_finish_early_strategy.rs | 26 ++++++++ .../signatures_outcome.rs | 36 ++++++++++- .../test_parallel_interactor.rs | 2 +- tests/main.rs | 61 ++++++++++++++++++- 4 files changed, 121 insertions(+), 4 deletions(-) diff --git a/src/signing/collector/signing_finish_early_strategy.rs b/src/signing/collector/signing_finish_early_strategy.rs index da8be37e..afd87f35 100644 --- a/src/signing/collector/signing_finish_early_strategy.rs +++ b/src/signing/collector/signing_finish_early_strategy.rs @@ -34,4 +34,30 @@ impl SigningFinishEarlyStrategy { when_some_transaction_is_invalid, } } + + pub fn r#continue() -> Self { + Self::new( + WhenAllTransactionsAreValid(SignaturesCollectingContinuation::Continue), + WhenSomeTransactionIsInvalid(SignaturesCollectingContinuation::Continue), + ) + } +} + +#[cfg(test)] +mod tests { + use super::*; + type Sut = SigningFinishEarlyStrategy; + + #[test] + fn test_continue() { + let sut = Sut::r#continue(); + assert_eq!( + sut.when_all_transactions_are_valid.0, + SignaturesCollectingContinuation::Continue + ); + assert_eq!( + sut.when_some_transaction_is_invalid.0, + SignaturesCollectingContinuation::Continue + ); + } } diff --git a/src/signing/signatures_outcome_types/signatures_outcome.rs b/src/signing/signatures_outcome_types/signatures_outcome.rs index ccf21df8..fa0acac7 100644 --- a/src/signing/signatures_outcome_types/signatures_outcome.rs +++ b/src/signing/signatures_outcome_types/signatures_outcome.rs @@ -56,6 +56,8 @@ impl SignaturesOutcome { "Discrepancy, found intent hash in both successful and failed transactions, this is a programmer error." ); + assert!(failed_transactions.is_empty() || !neglected_factor_sources.is_empty(), "Discrepancy, found failed transactions but no neglected factor sources, this is a programmer error."); + Self { successful_transactions, failed_transactions, @@ -83,13 +85,33 @@ impl SignaturesOutcome { self.neglected_factor_sources.clone() } - pub fn ids_of_neglected_factor_sources(&self) -> IndexSet { + fn ids_of_neglected_factor_sources_filter( + &self, + filter: fn(&NeglectedFactor) -> bool, + ) -> IndexSet { self.neglected_factor_sources() .into_iter() + .filter(filter) .map(|n| n.factor_source_id()) .collect() } + pub fn ids_of_neglected_factor_sources(&self) -> IndexSet { + self.ids_of_neglected_factor_sources_filter(|_| true) + } + + pub fn ids_of_neglected_factor_sources_skipped_by_user( + &self, + ) -> IndexSet { + self.ids_of_neglected_factor_sources_filter(|nf| { + nf.reason == NeglectFactorReason::UserExplicitlySkipped + }) + } + + pub fn ids_of_neglected_factor_sources_failed(&self) -> IndexSet { + self.ids_of_neglected_factor_sources_filter(|nf| nf.reason == NeglectFactorReason::Failure) + } + pub fn signatures_of_failed_transactions(&self) -> IndexSet { self.failed_transactions.all_signatures() } @@ -120,4 +142,16 @@ mod tests { [], ); } + + #[test] + #[should_panic( + expected = "Discrepancy, found failed transactions but no neglected factor sources, this is a programmer error." + )] + fn new_panics_if_failed_tx_is_not_empty_but_neglected_is() { + Sut::new( + MaybeSignedTransactions::empty(), + MaybeSignedTransactions::sample(), + [], + ); + } } diff --git a/src/testing/signing/test_interactors/test_parallel_interactor.rs b/src/testing/signing/test_interactors/test_parallel_interactor.rs index bbe5a009..2f551333 100644 --- a/src/testing/signing/test_interactors/test_parallel_interactor.rs +++ b/src/testing/signing/test_interactors/test_parallel_interactor.rs @@ -27,7 +27,7 @@ impl SignWithFactorParallelInteractor for TestSigningParallelInteractor { .collect::>(); if self.should_simulate_failure(ids.clone()) { - return SignWithFactorsOutcome::user_skipped_factors(ids); + return SignWithFactorsOutcome::failure_with_factors(ids); } match self diff --git a/tests/main.rs b/tests/main.rs index 4db58b1a..39be90f1 100644 --- a/tests/main.rs +++ b/tests/main.rs @@ -738,6 +738,7 @@ mod signing_tests { #[actix_rt::test] async fn many_failing_tx() { + sensible_env_logger::safe_init!(); let factor_sources = &HDFactorSource::all(); let a0 = &Account::a0(); let p3 = &Persona::p3(); @@ -778,6 +779,18 @@ mod signing_tests { .collect_vec() ); + assert_eq!( + outcome + .ids_of_neglected_factor_sources_failed() + .into_iter() + .collect_vec(), + vec![FactorSourceIDFromHash::fs0()] + ); + + assert!(outcome + .ids_of_neglected_factor_sources_skipped_by_user() + .is_empty()); + assert_eq!( outcome .successful_transactions() @@ -1226,6 +1239,40 @@ mod signing_tests { ); let outcome = collector.collect_signatures().await; assert!(!outcome.successful()); + assert_eq!( + outcome + .ids_of_neglected_factor_sources_failed() + .into_iter() + .collect_vec(), + vec![FactorSourceIDFromHash::fs0()] + ); + assert!(outcome + .ids_of_neglected_factor_sources_skipped_by_user() + .is_empty()) + } + + async fn failure_e5() { + let collector = SignaturesCollector::new_test( + SigningFinishEarlyStrategy::r#continue(), + HDFactorSource::all(), + [TXToSign::new([E::e5()])], + SimulatedUser::prudent_with_failures( + SimulatedFailures::with_simulated_failures([FactorSourceIDFromHash::fs4()]), + ), + ); + + let outcome = collector.collect_signatures().await; + assert!(outcome.successful()); + assert_eq!( + outcome + .ids_of_neglected_factor_sources_failed() + .into_iter() + .collect_vec(), + vec![FactorSourceIDFromHash::fs4()] + ); + assert!(outcome + .ids_of_neglected_factor_sources_skipped_by_user() + .is_empty()); } async fn building_can_succeed_even_if_one_factor_source_fails_assert_ids_of_successful_tx_e4< @@ -1422,10 +1469,15 @@ mod signing_tests { } #[actix_rt::test] - async fn failure() { + async fn failure_a0() { failure_e0::().await } + #[actix_rt::test] + async fn failure_a5() { + failure_e5::().await + } + #[actix_rt::test] async fn building_can_succeed_even_if_one_factor_source_fails_assert_ids_of_successful_tx( ) { @@ -1597,10 +1649,15 @@ mod signing_tests { } #[actix_rt::test] - async fn failure() { + async fn failure_p0() { failure_e0::().await } + #[actix_rt::test] + async fn failure_p5() { + failure_e5::().await + } + #[actix_rt::test] async fn building_can_succeed_even_if_one_factor_source_fails_assert_ids_of_successful_tx( ) {