From 422fb1be8272c7da8a4c664c4624b69b52d50514 Mon Sep 17 00:00:00 2001 From: Alexander Cyon Date: Wed, 28 Aug 2024 16:04:20 +0200 Subject: [PATCH 1/6] neglected --- src/signing/collector/signatures_collector.rs | 51 +++++------ .../parallel_batch_signing_request.rs | 6 +- .../serial_batch_signing_request.rs | 10 +-- .../sign_with_factor_parallel_interactor.rs | 5 +- .../sign_with_factor_serial_interactor.rs | 5 +- src/signing/petition_types/petition_entity.rs | 69 ++++++++------- .../factor_source_referencing.rs | 19 ++++ .../petition_factors_types/mod.rs | 4 + .../neglected_factor_instance.rs | 68 +++++++++++++++ .../petition_factors.rs | 31 ++++--- .../petition_factors_state.rs | 66 +++++++------- .../petition_factors_state_snapshot.rs | 34 ++++---- .../petition_factors_sub_state.rs | 25 +----- .../petition_types/petition_of_transaction.rs | 41 +++++---- src/signing/petition_types/petitions.rs | 86 +++++++++++-------- .../signatures_outcome.rs | 26 ++++-- src/testing/signing/simulated_user.rs | 2 +- .../test_parallel_interactor.rs | 28 +++--- .../test_serial_interactor.rs | 20 ++--- src/types/factor_sources_of_kind.rs | 7 -- ...rs => invalid_transaction_if_neglected.rs} | 16 ++-- src/types/mod.rs | 4 +- ...n_with_factor_source_or_sources_outcome.rs | 37 ++++---- tests/main.rs | 26 +++--- 24 files changed, 397 insertions(+), 289 deletions(-) create mode 100644 src/signing/petition_types/petition_factors_types/factor_source_referencing.rs create mode 100644 src/signing/petition_types/petition_factors_types/neglected_factor_instance.rs rename src/types/{invalid_transaction_if_skipped.rs => invalid_transaction_if_neglected.rs} (85%) diff --git a/src/signing/collector/signatures_collector.rs b/src/signing/collector/signatures_collector.rs index 389ea6a0..7992be38 100644 --- a/src/signing/collector/signatures_collector.rs +++ b/src/signing/collector/signatures_collector.rs @@ -144,7 +144,7 @@ impl SignaturesCollector { Continue } - async fn use_factor_sources(&self, factor_sources_of_kind: &FactorSourcesOfKind) -> Result<()> { + async fn sign_with_factors_of_kind(&self, factor_sources_of_kind: FactorSourcesOfKind) { let interactor = self .dependencies .interactors @@ -161,15 +161,15 @@ impl SignaturesCollector { .map(|f| f.factor_source_id()) .collect(), ); - if !request.invalid_transactions_if_skipped.is_empty() { + if !request.invalid_transactions_if_neglected.is_empty() { info!( - "If factors {:?} are skipped, invalid TXs: {:?}", + "If factors {:?} are neglected, invalid TXs: {:?}", request.per_factor_source.keys(), - request.invalid_transactions_if_skipped + request.invalid_transactions_if_neglected ) } debug!("Dispatching parallel request to interactor: {:?}", request); - let response = interactor.sign(request).await?; + let response = interactor.sign(request).await; debug!("Got response from parallel interactor: {:?}", response); self.process_batch_response(response); } @@ -185,16 +185,17 @@ impl SignaturesCollector { let request = self.request_for_serial_interactor(&factor_source.factor_source_id()); - if !request.invalid_transactions_if_skipped.is_empty() { + if !request.invalid_transactions_if_neglected.is_empty() { info!( - "If factor {:?} are skipped, invalid TXs: {:?}", - request.input.factor_source_id, request.invalid_transactions_if_skipped + "If factor {:?} are neglected, invalid TXs: {:?}", + request.input.factor_source_id, + request.invalid_transactions_if_neglected ) } debug!("Dispatching serial request to interactor: {:?}", request); // Produce the results from the interactor - let response = interactor.sign(request).await?; + let response = interactor.sign(request).await; debug!("Got response from serial interactor: {:?}", response); // Report the results back to the collector @@ -206,17 +207,6 @@ impl SignaturesCollector { } } } - Ok(()) - } - - async fn sign_with_factors_of_kind(&self, factor_sources_of_kind: FactorSourcesOfKind) { - let Err(e) = self.use_factor_sources(&factor_sources_of_kind).await else { - return; - }; - error!("Failure using factor {:?}", e); - self.process_batch_response(SignWithFactorSourceOrSourcesOutcome::Skipped { - ids_of_skipped_factors_sources: factor_sources_of_kind.factor_source_ids(), - }) } /// In decreasing "friction order" @@ -256,7 +246,7 @@ impl SignaturesCollector { SerialBatchSigningRequest::new( batch_signing_request, - self.invalid_transactions_if_skipped_factor_sources(IndexSet::from_iter([ + self.invalid_transactions_if_neglected_factor_sources(IndexSet::from_iter([ *factor_source_id, ])) .into_iter() @@ -274,27 +264,30 @@ impl SignaturesCollector { .map(|fid| (*fid, self.input_for_interactor(fid))) .collect::>(); - let invalid_transactions_if_skipped = - self.invalid_transactions_if_skipped_factor_sources(factor_source_ids); + let invalid_transactions_if_neglected = + self.invalid_transactions_if_neglected_factor_sources(factor_source_ids); - info!("Invalid if skipped: {:?}", invalid_transactions_if_skipped); + info!( + "Invalid if neglected: {:?}", + invalid_transactions_if_neglected + ); // Prepare the request for the interactor - ParallelBatchSigningRequest::new(per_factor_source, invalid_transactions_if_skipped) + ParallelBatchSigningRequest::new(per_factor_source, invalid_transactions_if_neglected) } - fn invalid_transactions_if_skipped_factor_sources( + fn invalid_transactions_if_neglected_factor_sources( &self, factor_source_ids: IndexSet, - ) -> IndexSet { + ) -> IndexSet { self.state .borrow() .petitions .borrow() - .invalid_transactions_if_skipped_factors(factor_source_ids) + .invalid_transactions_if_neglected_factors(factor_source_ids) } - fn process_batch_response(&self, response: SignWithFactorSourceOrSourcesOutcome) { + fn process_batch_response(&self, response: SignWithFactorsOutcome) { let state = self.state.borrow_mut(); let petitions = state.petitions.borrow_mut(); petitions.process_batch_response(response) diff --git a/src/signing/interactors/parallel_batch_signing_request.rs b/src/signing/interactors/parallel_batch_signing_request.rs index c9d240d1..2834352c 100644 --- a/src/signing/interactors/parallel_batch_signing_request.rs +++ b/src/signing/interactors/parallel_batch_signing_request.rs @@ -11,17 +11,17 @@ pub struct ParallelBatchSigningRequest { /// A collection of transactions which would be invalid if the user skips /// signing with this factor source. - pub invalid_transactions_if_skipped: IndexSet, + pub invalid_transactions_if_neglected: IndexSet, } impl ParallelBatchSigningRequest { pub fn new( per_factor_source: IndexMap, - invalid_transactions_if_skipped: IndexSet, + invalid_transactions_if_neglected: IndexSet, ) -> Self { Self { per_factor_source, - invalid_transactions_if_skipped, + invalid_transactions_if_neglected, } } } diff --git a/src/signing/interactors/serial_batch_signing_request.rs b/src/signing/interactors/serial_batch_signing_request.rs index 54731716..671093b6 100644 --- a/src/signing/interactors/serial_batch_signing_request.rs +++ b/src/signing/interactors/serial_batch_signing_request.rs @@ -3,24 +3,24 @@ use crate::prelude::*; /// A batch signing request used with a SignWithFactorSerialInteractor, containing /// a collection of transactions to sign with multiple keys (derivation paths), /// and a collection of transactions which would be invalid if the user skips -/// signing with this factor source. +/// signing with this factor source, or if we fail to sign. #[derive(derive_more::Debug)] #[debug("input: {:#?}", input)] pub struct SerialBatchSigningRequest { pub input: BatchTXBatchKeySigningRequest, /// A collection of transactions which would be invalid if the user skips - /// signing with this factor source. - pub invalid_transactions_if_skipped: Vec, + /// signing with this factor source, or if we fail to sign + pub invalid_transactions_if_neglected: Vec, } impl SerialBatchSigningRequest { pub fn new( input: BatchTXBatchKeySigningRequest, - invalid_transactions_if_skipped: Vec, + invalid_transactions_if_neglected: Vec, ) -> Self { Self { input, - invalid_transactions_if_skipped, + invalid_transactions_if_neglected, } } } diff --git a/src/signing/interactors/sign_with_factor_parallel_interactor.rs b/src/signing/interactors/sign_with_factor_parallel_interactor.rs index 2a6b6caa..bd82d137 100644 --- a/src/signing/interactors/sign_with_factor_parallel_interactor.rs +++ b/src/signing/interactors/sign_with_factor_parallel_interactor.rs @@ -20,8 +20,5 @@ use crate::prelude::*; /// Example of a Parallel Batch Signing Driver is that for DeviceFactorSource. #[async_trait::async_trait] pub trait SignWithFactorParallelInteractor { - async fn sign( - &self, - request: ParallelBatchSigningRequest, - ) -> Result; + async fn sign(&self, request: ParallelBatchSigningRequest) -> SignWithFactorsOutcome; } diff --git a/src/signing/interactors/sign_with_factor_serial_interactor.rs b/src/signing/interactors/sign_with_factor_serial_interactor.rs index 1946bf73..72052aaa 100644 --- a/src/signing/interactors/sign_with_factor_serial_interactor.rs +++ b/src/signing/interactors/sign_with_factor_serial_interactor.rs @@ -16,8 +16,5 @@ use crate::prelude::*; /// might not even even allow multiple SecurityQuestionsFactorSources to be used). #[async_trait::async_trait] pub trait SignWithFactorSerialInteractor { - async fn sign( - &self, - request: SerialBatchSigningRequest, - ) -> Result; + async fn sign(&self, request: SerialBatchSigningRequest) -> SignWithFactorsOutcome; } diff --git a/src/signing/petition_types/petition_entity.rs b/src/signing/petition_types/petition_entity.rs index e99d7ba2..471bcf7c 100644 --- a/src/signing/petition_types/petition_entity.rs +++ b/src/signing/petition_types/petition_entity.rs @@ -94,14 +94,14 @@ impl PetitionEntity { .collect::>() } - pub fn all_skipped_factor_instance(&self) -> IndexSet { - self.union_of(|f| f.all_skipped()) + pub fn all_neglected_factor_instance(&self) -> IndexSet { + self.union_of(|f| f.all_neglected()) } - pub fn all_skipped_factor_sources(&self) -> IndexSet { - self.all_skipped_factor_instance() + pub fn all_neglected_factor_sources(&self) -> IndexSet { + self.all_neglected_factor_instance() .into_iter() - .map(|f| f.factor_source_id) + .map(|n| n.as_neglected_factor()) .collect::>() } @@ -143,12 +143,12 @@ impl PetitionEntity { self.both(r#do, |_, _| ()) } - pub fn skipped_factor_source_if_relevant(&self, factor_source_id: &FactorSourceIDFromHash) { - self.both_void(|l| l.skip_if_references(factor_source_id, true)); + pub fn neglected_factor_source_if_relevant(&self, neglected: NeglectedFactor) { + self.both_void(|l| l.neglect_if_references(neglected.clone(), true)); } /// # Panics - /// Panics if this factor source has already been skipped or signed with. + /// Panics if this factor source has already been neglected or signed with. /// /// Or panics if the factor source is not known to this petition. pub fn add_signature(&self, signature: HDSignature) { @@ -164,17 +164,17 @@ impl PetitionEntity { }) } - pub fn invalid_transactions_if_skipped_factors( + pub fn invalid_transactions_if_neglected_factors( &self, factor_source_ids: IndexSet, - ) -> IndexSet { - let skip_status = self.status_if_skipped_factors(factor_source_ids); - match skip_status { + ) -> IndexSet { + let status_if_neglected = self.status_if_neglected_factors(factor_source_ids); + match status_if_neglected { PetitionFactorsStatus::Finished(finished_reason) => match finished_reason { PetitionFactorsStatusFinished::Fail => { let intent_hash = self.intent_hash.clone(); let invalid_transaction = - InvalidTransactionIfSkipped::new(intent_hash, vec![self.entity.clone()]); + InvalidTransactionIfNeglected::new(intent_hash, vec![self.entity.clone()]); IndexSet::from_iter([invalid_transaction]) } PetitionFactorsStatusFinished::Success => IndexSet::new(), @@ -183,25 +183,27 @@ impl PetitionEntity { } } - pub fn status_if_skipped_factors( + pub fn status_if_neglected_factors( &self, factor_source_ids: IndexSet, ) -> PetitionFactorsStatus { let simulation = self.clone(); for factor_source_id in factor_source_ids.iter() { simulation - .did_skip_if_relevant(factor_source_id, true) + .neglect_if_relevant( + NeglectedFactor::new( + NeglectFactorReason::UserExplicitlySkipped, + *factor_source_id, + ), + true, + ) .unwrap(); } simulation.status() } - pub fn did_skip_if_relevant( - &self, - factor_source_id: &FactorSourceIDFromHash, - simulated: bool, - ) -> Result<()> { - self.both_void(|p| p.did_skip_if_relevant(factor_source_id, simulated)); + pub fn neglect_if_relevant(&self, neglected: NeglectedFactor, simulated: bool) -> Result<()> { + self.both_void(|p| p.neglect_if_relevant(neglected.clone(), simulated)); Ok(()) } @@ -299,7 +301,7 @@ mod tests { let entity = AddressOfAccountOrPersona::Account(AccountAddress::sample()); let tx = IntentHash::sample_third(); let sut = Sut::new_securified(tx.clone(), entity.clone(), matrix); - let invalid = sut.invalid_transactions_if_skipped_factors(IndexSet::from_iter([ + let invalid = sut.invalid_transactions_if_neglected_factors(IndexSet::from_iter([ d0.factor_source_id(), d1.factor_source_id(), ])); @@ -337,8 +339,9 @@ mod tests { let entity = AddressOfAccountOrPersona::Account(AccountAddress::sample()); let tx = IntentHash::sample_third(); let sut = Sut::new_securified(tx.clone(), entity.clone(), matrix); - let invalid = sut - .invalid_transactions_if_skipped_factors(IndexSet::from_iter([d0.factor_source_id()])); + let invalid = sut.invalid_transactions_if_neglected_factors(IndexSet::from_iter([ + d0.factor_source_id() + ])); assert!(invalid.is_empty()); } @@ -362,7 +365,7 @@ mod tests { let entity = AddressOfAccountOrPersona::Account(AccountAddress::sample()); let tx = IntentHash::sample_third(); let sut = Sut::new_securified(tx.clone(), entity.clone(), matrix); - let invalid = sut.invalid_transactions_if_skipped_factors(IndexSet::from_iter([ + let invalid = sut.invalid_transactions_if_neglected_factors(IndexSet::from_iter([ d0.factor_source_id(), d1.factor_source_id(), ])); @@ -404,8 +407,9 @@ mod tests { let tx = IntentHash::sample_third(); let sut = Sut::new_securified(tx.clone(), entity.clone(), matrix); - let invalid = sut - .invalid_transactions_if_skipped_factors(IndexSet::from_iter([d1.factor_source_id()])); + let invalid = sut.invalid_transactions_if_neglected_factors(IndexSet::from_iter([ + d1.factor_source_id() + ])); assert_eq!( invalid @@ -445,15 +449,16 @@ mod tests { let tx = IntentHash::sample_third(); let sut = Sut::new_securified(tx.clone(), entity.clone(), matrix); - let invalid = sut - .invalid_transactions_if_skipped_factors(IndexSet::from_iter([d1.factor_source_id()])); + let invalid = sut.invalid_transactions_if_neglected_factors(IndexSet::from_iter([ + d1.factor_source_id() + ])); assert!(invalid.is_empty()); } #[test] fn debug() { - pretty_assertions::assert_eq!(format!("{:?}", Sut::sample()), "intent_hash: TXID(\"dedede\"), entity: acco_Grace, \"threshold_factors PetitionFactors(input: PetitionFactorsInput(factors: {\\n factor_source_id: Device:00, derivation_path: 0/A/tx/6,\\n factor_source_id: Arculus:03, derivation_path: 0/A/tx/6,\\n factor_source_id: Yubikey:05, derivation_path: 0/A/tx/6,\\n}), state_snapshot: signatures: \\\"\\\", skipped: \\\"\\\")\"\"override_factors PetitionFactors(input: PetitionFactorsInput(factors: {\\n factor_source_id: Ledger:01, derivation_path: 0/A/tx/6,\\n factor_source_id: Arculus:04, derivation_path: 0/A/tx/6,\\n}), state_snapshot: signatures: \\\"\\\", skipped: \\\"\\\")\""); + pretty_assertions::assert_eq!(format!("{:?}", Sut::sample()), "intent_hash: TXID(\"dedede\"), entity: acco_Grace, \"threshold_factors PetitionFactors(input: PetitionFactorsInput(factors: {\\n factor_source_id: Device:00, derivation_path: 0/A/tx/6,\\n factor_source_id: Arculus:03, derivation_path: 0/A/tx/6,\\n factor_source_id: Yubikey:05, derivation_path: 0/A/tx/6,\\n}), state_snapshot: signatures: \\\"\\\", neglected: \\\"\\\")\"\"override_factors PetitionFactors(input: PetitionFactorsInput(factors: {\\n factor_source_id: Ledger:01, derivation_path: 0/A/tx/6,\\n factor_source_id: Arculus:04, derivation_path: 0/A/tx/6,\\n}), state_snapshot: signatures: \\\"\\\", neglected: \\\"\\\")\""); } #[test] @@ -517,7 +522,7 @@ mod tests { } #[test] - fn invalid_transactions_if_skipped_success() { + fn invalid_transactions_if_neglected_success() { let sut = Sut::sample(); sut.add_signature(HDSignature::produced_signing_with_input( HDSignatureInput::new( @@ -535,7 +540,7 @@ mod tests { assert!(sut // Already signed with override factor `FactorSourceIDFromHash::fs1()`. Thus // can skip - .invalid_transactions_if_skipped_factors(IndexSet::from_iter([f])) + .invalid_transactions_if_neglected_factors(IndexSet::from_iter([f])) .is_empty()) }; can_skip(FactorSourceIDFromHash::fs0()); diff --git a/src/signing/petition_types/petition_factors_types/factor_source_referencing.rs b/src/signing/petition_types/petition_factors_types/factor_source_referencing.rs new file mode 100644 index 00000000..14ad5024 --- /dev/null +++ b/src/signing/petition_types/petition_factors_types/factor_source_referencing.rs @@ -0,0 +1,19 @@ +use crate::prelude::*; + +pub trait FactorSourceReferencing: std::hash::Hash + PartialEq + Eq + Clone { + fn factor_source_id(&self) -> FactorSourceIDFromHash; +} + +impl FactorSourceReferencing for HierarchicalDeterministicFactorInstance { + fn factor_source_id(&self) -> FactorSourceIDFromHash { + self.factor_source_id + } +} + +impl FactorSourceReferencing for HDSignature { + fn factor_source_id(&self) -> FactorSourceIDFromHash { + self.owned_factor_instance() + .factor_instance() + .factor_source_id + } +} diff --git a/src/signing/petition_types/petition_factors_types/mod.rs b/src/signing/petition_types/petition_factors_types/mod.rs index 56a3a5c8..e8527b90 100644 --- a/src/signing/petition_types/petition_factors_types/mod.rs +++ b/src/signing/petition_types/petition_factors_types/mod.rs @@ -1,3 +1,5 @@ +mod factor_source_referencing; +mod neglected_factor_instance; mod petition_factors; mod petition_factors_input; mod petition_factors_state; @@ -10,5 +12,7 @@ use petition_factors_state::*; use petition_factors_state_snapshot::*; use petition_factors_sub_state::*; +pub use factor_source_referencing::*; +pub use neglected_factor_instance::*; pub use petition_factors::*; pub use petition_factors_status::*; diff --git a/src/signing/petition_types/petition_factors_types/neglected_factor_instance.rs b/src/signing/petition_types/petition_factors_types/neglected_factor_instance.rs new file mode 100644 index 00000000..b9ab2a55 --- /dev/null +++ b/src/signing/petition_types/petition_factors_types/neglected_factor_instance.rs @@ -0,0 +1,68 @@ +use crate::prelude::*; + +impl NeglectedFactorInstance { + pub fn as_neglected_factor(&self) -> NeglectedFactor { + NeglectedFactor::new(self.reason, self.factor_source_id()) + } +} +impl FactorSourceReferencing for NeglectedFactorInstance { + fn factor_source_id(&self) -> FactorSourceIDFromHash { + self.content.factor_source_id() + } +} + +impl FactorSourceReferencing for NeglectedFactor { + fn factor_source_id(&self) -> FactorSourceIDFromHash { + self.content + } +} + +impl HasSampleValues for NeglectedFactorInstance { + fn sample() -> Self { + Self::new( + NeglectFactorReason::UserExplicitlySkipped, + HierarchicalDeterministicFactorInstance::sample(), + ) + } + fn sample_other() -> Self { + Self::new( + NeglectFactorReason::Failure, + HierarchicalDeterministicFactorInstance::sample_other(), + ) + } +} + +pub type NeglectedFactor = AbstractNeglectedFactor; +pub type NeglectedFactors = AbstractNeglectedFactor>; +pub type NeglectedFactorInstance = AbstractNeglectedFactor; + +#[derive(Clone, PartialEq, Eq, Hash)] +pub struct AbstractNeglectedFactor { + pub reason: NeglectFactorReason, + pub content: T, +} +impl AbstractNeglectedFactor { + pub fn new(reason: NeglectFactorReason, content: T) -> Self { + Self { reason, content } + } +} + +impl std::fmt::Debug for AbstractNeglectedFactor { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Neglected") + .field("reason", &self.reason) + .field("content", &self.content) + .finish() + } +} + +#[derive(Clone, Copy, PartialEq, Eq, Hash, derive_more::Debug, derive_more::Display)] +pub enum NeglectFactorReason { + #[display("User Skipped")] + #[debug("UserExplicitlySkipped")] + UserExplicitlySkipped, + + #[display("Failure")] + #[debug("Failure")] + Failure, +} diff --git a/src/signing/petition_types/petition_factors_types/petition_factors.rs b/src/signing/petition_types/petition_factors_types/petition_factors.rs index a9e66f5c..7bd713f6 100644 --- a/src/signing/petition_types/petition_factors_types/petition_factors.rs +++ b/src/signing/petition_types/petition_factors_types/petition_factors.rs @@ -38,8 +38,8 @@ impl PetitionFactors { self.input.factors.clone() } - pub fn all_skipped(&self) -> IndexSet { - self.state.borrow().all_skipped() + pub fn all_neglected(&self) -> IndexSet { + self.state.borrow().all_neglected() } pub fn all_signatures(&self) -> IndexSet { @@ -73,13 +73,14 @@ impl PetitionFactors { )) } - pub fn did_skip_if_relevant(&self, factor_source_id: &FactorSourceIDFromHash, simulated: bool) { + pub fn neglect_if_relevant(&self, neglected: NeglectedFactor, simulated: bool) { + let factor_source_id = &neglected.factor_source_id(); if let Some(_x_) = self.reference_to_factor_source_with_id(factor_source_id) { debug!( - "PetitionFactors = kind {:?} skip factor source with id: {}, simulated: {}", - self.factor_list_kind, factor_source_id, simulated + "PetitionFactors = kind {:?} neglect factor source with id: {}, reason: {}, simulated: {}", + self.factor_list_kind, factor_source_id, neglected.reason, simulated ); - self.did_skip(factor_source_id, simulated) + self.neglect(neglected, simulated) } else { debug!( "PetitionFactors = kind {:?} did not reference factor source with id: {}", @@ -88,9 +89,13 @@ impl PetitionFactors { } } - fn did_skip(&self, factor_source_id: &FactorSourceIDFromHash, simulated: bool) { - let factor_instance = self.expect_reference_to_factor_source_with_id(factor_source_id); - self.state.borrow_mut().did_skip(factor_instance, simulated); + fn neglect(&self, neglected: NeglectedFactor, simulated: bool) { + let factor_instance = + self.expect_reference_to_factor_source_with_id(&neglected.factor_source_id()); + self.state.borrow_mut().neglect( + &NeglectedFactorInstance::new(neglected.reason, factor_instance.clone()), + simulated, + ); } pub fn has_owned_instance_with_id(&self, owned_factor_instance: &OwnedFactorInstance) -> bool { @@ -114,7 +119,7 @@ impl PetitionFactors { } /// # Panics - /// Panics if this factor source has already been skipped or signed with. + /// Panics if this factor source has already been neglected or signed with. fn add_signature(&self, signature: &HDSignature) { let state = self.state.borrow_mut(); state.add_signature(signature) @@ -128,9 +133,9 @@ impl PetitionFactors { .is_some() } - pub fn skip_if_references(&self, factor_source_id: &FactorSourceIDFromHash, simulated: bool) { - if self.references_factor_source_with_id(factor_source_id) { - self.did_skip(factor_source_id, simulated) + pub fn neglect_if_references(&self, neglected: NeglectedFactor, simulated: bool) { + if self.references_factor_source_with_id(&neglected.factor_source_id()) { + self.neglect(neglected, simulated) } } diff --git a/src/signing/petition_types/petition_factors_types/petition_factors_state.rs b/src/signing/petition_types/petition_factors_types/petition_factors_state.rs index 255e7dfd..8b549901 100644 --- a/src/signing/petition_types/petition_factors_types/petition_factors_state.rs +++ b/src/signing/petition_types/petition_factors_types/petition_factors_state.rs @@ -4,14 +4,16 @@ use super::*; use crate::prelude::*; /// Mutable state of `PetitionFactors`, keeping track of which factors that -/// have either signed or been skipped. -#[derive(Clone, PartialEq, Eq, Debug)] +/// have either signed or been neglected. +#[derive(Clone, PartialEq, Eq, derive_more::Debug)] +#[debug("PetitionFactorsState(signed: {:?}, neglected: {:?})", signed.borrow().clone(), neglected.borrow().clone())] pub struct PetitionFactorsState { /// Factors that have signed. signed: RefCell>, - /// Factors that user skipped. - skipped: RefCell>, + /// Neglected factors, either due to user explicitly skipping, or due + /// implicitly neglected to failure. + neglected: RefCell>, } impl PetitionFactorsState { @@ -19,15 +21,13 @@ impl PetitionFactorsState { pub(super) fn new() -> Self { Self { signed: RefCell::new(PetitionFactorsSubState::<_>::new()), - skipped: RefCell::new(PetitionFactorsSubState::<_>::new()), + neglected: RefCell::new(PetitionFactorsSubState::<_>::new()), } } - /// A reference to the skipped factors so far. - pub(super) fn skipped( - &self, - ) -> Ref> { - self.skipped.borrow() + /// A reference to the neglected factors so far. + pub(super) fn neglected(&self) -> Ref> { + self.neglected.borrow() } /// A reference to the factors which have been signed with so far. @@ -40,13 +40,13 @@ impl PetitionFactorsState { self.signed().snapshot() } - /// A set factors have been skipped so far. - pub fn all_skipped(&self) -> IndexSet { - self.skipped().snapshot() + /// A set factors have been neglected so far. + pub fn all_neglected(&self) -> IndexSet { + self.neglected().snapshot() } /// # Panics - /// Panics if this factor source has already been skipped or signed with. + /// Panics if this factor source has already been neglected or signed with. fn assert_not_referencing_factor_source(&self, factor_source_id: FactorSourceIDFromHash) { assert!( !self.references_factor_source_by_id(factor_source_id), @@ -56,35 +56,31 @@ impl PetitionFactorsState { } /// # Panics - /// Panics if this factor source has already been skipped or signed and + /// Panics if this factor source has already been neglected or signed and /// this is not a simulation. - pub(crate) fn did_skip( - &self, - factor_instance: &HierarchicalDeterministicFactorInstance, - simulated: bool, - ) { + pub(crate) fn neglect(&self, neglected: &NeglectedFactorInstance, simulated: bool) { if !simulated { - self.assert_not_referencing_factor_source(factor_instance.factor_source_id); + self.assert_not_referencing_factor_source(neglected.factor_source_id()); } - self.skipped.borrow_mut().insert(factor_instance); + self.neglected.borrow_mut().insert(neglected); } /// # Panics - /// Panics if this factor source has already been skipped or signed with. + /// Panics if this factor source has already been neglected or signed with. pub(crate) fn add_signature(&self, signature: &HDSignature) { self.assert_not_referencing_factor_source(signature.factor_source_id()); self.signed.borrow_mut().insert(signature) } pub(super) fn snapshot(&self) -> PetitionFactorsStateSnapshot { - PetitionFactorsStateSnapshot::new(self.signed().snapshot(), self.skipped().snapshot()) + PetitionFactorsStateSnapshot::new(self.signed().snapshot(), self.neglected().snapshot()) } fn references_factor_source_by_id(&self, factor_source_id: FactorSourceIDFromHash) -> bool { self.signed() .references_factor_source_by_id(factor_source_id) || self - .skipped() + .neglected() .references_factor_source_by_id(factor_source_id) } } @@ -96,13 +92,25 @@ mod tests { type Sut = PetitionFactorsState; + impl PetitionFactorsState { + fn skip(&self, id: &HierarchicalDeterministicFactorInstance, simulated: bool) { + self.neglect( + &NeglectedFactorInstance::new( + NeglectFactorReason::UserExplicitlySkipped, + id.clone(), + ), + simulated, + ) + } + } + #[test] #[should_panic] fn skipping_twice_panics() { let sut = Sut::new(); let fi = HierarchicalDeterministicFactorInstance::sample(); - sut.did_skip(&fi, false); - sut.did_skip(&fi, false); + sut.skip(&fi, false); + sut.skip(&fi, false); } #[test] @@ -133,7 +141,7 @@ mod tests { sut.add_signature(&signature); - sut.did_skip(&factor_instance, false); + sut.skip(&factor_instance, false); } #[test] @@ -147,7 +155,7 @@ mod tests { FactorSourceIDFromHash::fs0(), ); - sut.did_skip(&factor_instance, false); + sut.skip(&factor_instance, false); let sign_input = HDSignatureInput::new( intent_hash, diff --git a/src/signing/petition_types/petition_factors_types/petition_factors_state_snapshot.rs b/src/signing/petition_types/petition_factors_types/petition_factors_state_snapshot.rs index bd46c01b..252ee17a 100644 --- a/src/signing/petition_types/petition_factors_types/petition_factors_state_snapshot.rs +++ b/src/signing/petition_types/petition_factors_types/petition_factors_state_snapshot.rs @@ -1,5 +1,7 @@ use crate::prelude::*; +use super::NeglectedFactorInstance; + /// An immutable "snapshot" of `PetitionFactorsState` #[derive(Clone, PartialEq, Eq, derive_more::Debug)] #[debug("{}", self.debug_str())] @@ -7,28 +9,28 @@ pub(super) struct PetitionFactorsStateSnapshot { /// Factors that have signed. signed: IndexSet, - /// Factors that user skipped. - skipped: IndexSet, + /// Factors that has been neglected. + neglected: IndexSet, } impl PetitionFactorsStateSnapshot { pub(super) fn new( signed: IndexSet, - skipped: IndexSet, + neglected: IndexSet, ) -> Self { - Self { signed, skipped } + Self { signed, neglected } } pub(super) fn prompted_count(&self) -> i8 { - self.signed_count() + self.skipped_count() + self.signed_count() + self.neglected_count() } pub(super) fn signed_count(&self) -> i8 { self.signed.len() as i8 } - fn skipped_count(&self) -> i8 { - self.skipped.len() as i8 + fn neglected_count(&self) -> i8 { + self.neglected.len() as i8 } #[allow(unused)] @@ -37,17 +39,17 @@ impl PetitionFactorsStateSnapshot { .signed .clone() .into_iter() - .map(|s| format!("{:#?}", s)) + .map(|s| format!("{:?}", s)) .join(", "); - let skipped = self - .skipped + let neglected = self + .neglected .clone() .into_iter() - .map(|s| format!("{:#?}", s)) + .map(|s| format!("{:?}", s)) .join(", "); - format!("signatures: {:#?}, skipped: {:#?}", signatures, skipped) + format!("signatures: {:#?}, neglected: {:#?}", signatures, neglected) } } @@ -56,15 +58,15 @@ impl HasSampleValues for PetitionFactorsStateSnapshot { Self::new( IndexSet::from_iter([HDSignature::sample(), HDSignature::sample_other()]), IndexSet::from_iter([ - HierarchicalDeterministicFactorInstance::sample(), - HierarchicalDeterministicFactorInstance::sample_other(), + NeglectedFactorInstance::sample(), + NeglectedFactorInstance::sample_other(), ]), ) } fn sample_other() -> Self { Self::new( IndexSet::from_iter([HDSignature::sample_other()]), - IndexSet::from_iter([HierarchicalDeterministicFactorInstance::sample_other()]), + IndexSet::from_iter([NeglectedFactorInstance::sample_other()]), ) } } @@ -88,6 +90,6 @@ mod tests { #[test] fn debug() { - assert_eq!(format!("{:?}", Sut::sample()), "signatures: \"HDSignature { input: HDSignatureInput { intent_hash: TXID(\\\"dedede\\\"), owned_factor_instance: acco_Alice: factor_source_id: Device:de, derivation_path: 0/A/tx/0 } }, HDSignature { input: HDSignatureInput { intent_hash: TXID(\\\"ababab\\\"), owned_factor_instance: ident_Alice: factor_source_id: Ledger:1e, derivation_path: 0/A/tx/1 } }\", skipped: \"factor_source_id: Device:de, derivation_path: 0/A/tx/0, factor_source_id: Ledger:1e, derivation_path: 0/A/tx/1\""); + assert_eq!(format!("{:?}", Sut::sample()), "signatures: \"HDSignature { input: HDSignatureInput { intent_hash: TXID(\\\"dedede\\\"), owned_factor_instance: acco_Alice: factor_source_id: Device:de, derivation_path: 0/A/tx/0 } }, HDSignature { input: HDSignatureInput { intent_hash: TXID(\\\"ababab\\\"), owned_factor_instance: ident_Alice: factor_source_id: Ledger:1e, derivation_path: 0/A/tx/1 } }\", neglected: \"Neglected { reason: UserExplicitlySkipped, content: factor_source_id: Device:de, derivation_path: 0/A/tx/0 }, Neglected { reason: Failure, content: factor_source_id: Ledger:1e, derivation_path: 0/A/tx/1 }\""); } } diff --git a/src/signing/petition_types/petition_factors_types/petition_factors_sub_state.rs b/src/signing/petition_types/petition_factors_types/petition_factors_sub_state.rs index bc26bd6c..51f38fd5 100644 --- a/src/signing/petition_types/petition_factors_types/petition_factors_sub_state.rs +++ b/src/signing/petition_types/petition_factors_types/petition_factors_sub_state.rs @@ -2,16 +2,17 @@ use crate::prelude::*; /// A sub-state of `PetitionFactorsState` which can be used to track factors /// that have signed or skipped. -#[derive(Clone, PartialEq, Eq, Debug)] +#[derive(Clone, PartialEq, Eq, derive_more::Debug)] +#[debug("[{}]", factors.borrow().clone().into_iter().map(|f| format!("{:?}", f)).join(", "))] pub struct PetitionFactorsSubState where - F: FactorSourceReferencing, + F: FactorSourceReferencing + std::fmt::Debug, { /// Factors that have signed or skipped factors: RefCell>, } -impl PetitionFactorsSubState { +impl PetitionFactorsSubState { pub(super) fn new() -> Self { Self { factors: RefCell::new(IndexSet::new()), @@ -36,21 +37,3 @@ impl PetitionFactorsSubState { .any(|sf| sf.factor_source_id() == factor_source_id) } } - -pub trait FactorSourceReferencing: std::hash::Hash + PartialEq + Eq + Clone { - fn factor_source_id(&self) -> FactorSourceIDFromHash; -} - -impl FactorSourceReferencing for HierarchicalDeterministicFactorInstance { - fn factor_source_id(&self) -> FactorSourceIDFromHash { - self.factor_source_id - } -} - -impl FactorSourceReferencing for HDSignature { - fn factor_source_id(&self) -> FactorSourceIDFromHash { - self.owned_factor_instance() - .factor_instance() - .factor_source_id - } -} diff --git a/src/signing/petition_types/petition_of_transaction.rs b/src/signing/petition_types/petition_of_transaction.rs index 1b18ffbe..0f667b50 100644 --- a/src/signing/petition_types/petition_of_transaction.rs +++ b/src/signing/petition_types/petition_of_transaction.rs @@ -11,6 +11,13 @@ pub(crate) struct PetitionTransaction { pub for_entities: RefCell>, } +#[derive(Clone, PartialEq, Eq)] +pub struct PetitionTransactionOutcome { + pub transaction_valid: bool, + pub signatures: IndexSet, + pub neglected_factors: IndexSet, +} + impl PetitionTransaction { pub(crate) fn new( intent_hash: IntentHash, @@ -33,13 +40,7 @@ impl PetitionTransaction { /// /// The third value in the tuple `(_, _, IndexSet)` contains the /// id of all the factor sources which was skipped. - pub fn outcome( - self, - ) -> ( - bool, - IndexSet, - IndexSet, - ) { + pub fn outcome(self) -> PetitionTransactionOutcome { let for_entities = self .for_entities .into_inner() @@ -47,7 +48,7 @@ impl PetitionTransaction { .map(|x| x.to_owned()) .collect_vec(); - let successful = for_entities + let transaction_valid = for_entities .iter() .all(|b| b.has_signatures_requirement_been_fulfilled()); @@ -56,12 +57,16 @@ impl PetitionTransaction { .flat_map(|x| x.all_signatures()) .collect::>(); - let skipped = for_entities + let neglected_factors = for_entities .iter() - .flat_map(|x| x.all_skipped_factor_sources()) - .collect::>(); + .flat_map(|x| x.all_neglected_factor_sources()) + .collect::>(); - (successful, signatures, skipped) + PetitionTransactionOutcome { + transaction_valid, + signatures, + neglected_factors, + } } fn _all_factor_instances(&self) -> IndexSet { @@ -90,10 +95,10 @@ impl PetitionTransaction { for_entity.add_signature(signature.clone()); } - pub fn skipped_factor_source(&self, factor_source_id: &FactorSourceIDFromHash) { + pub fn neglected_factor_source(&self, neglected: NeglectedFactor) { let mut for_entities = self.for_entities.borrow_mut(); for petition in for_entities.values_mut() { - petition.skipped_factor_source_if_relevant(factor_source_id) + petition.neglected_factor_source_if_relevant(neglected.clone()) } } @@ -108,15 +113,15 @@ impl PetitionTransaction { ) } - pub fn invalid_transactions_if_skipped_factors( + pub fn invalid_transactions_if_neglected_factors( &self, factor_source_ids: IndexSet, - ) -> IndexSet { + ) -> IndexSet { self.for_entities .borrow() .iter() .flat_map(|(_, petition)| { - petition.invalid_transactions_if_skipped_factors(factor_source_ids.clone()) + petition.invalid_transactions_if_neglected_factors(factor_source_ids.clone()) }) .collect() } @@ -190,6 +195,6 @@ mod tests { #[test] fn debug() { - assert_eq!(format!("{:?}", Sut::sample()), "PetitionTransaction(for_entities: [PetitionEntity(intent_hash: TXID(\"dedede\"), entity: acco_Grace, \"threshold_factors PetitionFactors(input: PetitionFactorsInput(factors: {\\n factor_source_id: Device:de, derivation_path: 0/A/tx/0,\\n factor_source_id: Ledger:1e, derivation_path: 0/A/tx/1,\\n}), state_snapshot: signatures: \\\"\\\", skipped: \\\"\\\")\"\"override_factors PetitionFactors(input: PetitionFactorsInput(factors: {\\n factor_source_id: Ledger:1e, derivation_path: 0/A/tx/1,\\n}), state_snapshot: signatures: \\\"\\\", skipped: \\\"\\\")\")])"); + assert_eq!(format!("{:?}", Sut::sample()), "PetitionTransaction(for_entities: [PetitionEntity(intent_hash: TXID(\"dedede\"), entity: acco_Grace, \"threshold_factors PetitionFactors(input: PetitionFactorsInput(factors: {\\n factor_source_id: Device:de, derivation_path: 0/A/tx/0,\\n factor_source_id: Ledger:1e, derivation_path: 0/A/tx/1,\\n}), state_snapshot: signatures: \\\"\\\", neglected: \\\"\\\")\"\"override_factors PetitionFactors(input: PetitionFactorsInput(factors: {\\n factor_source_id: Ledger:1e, derivation_path: 0/A/tx/1,\\n}), state_snapshot: signatures: \\\"\\\", neglected: \\\"\\\")\")])"); } } diff --git a/src/signing/petition_types/petitions.rs b/src/signing/petition_types/petitions.rs index e1184f65..c8196fb1 100644 --- a/src/signing/petition_types/petitions.rs +++ b/src/signing/petition_types/petitions.rs @@ -16,7 +16,7 @@ pub(crate) struct Petitions { /// /// Where A, B, C and D, all use the factor source, e.g. some arculus /// card which the user has setup as a factor (source) for all these accounts. - pub factor_to_txid: HashMap>, + pub factor_source_to_intent_hash: HashMap>, /// Lookup from TXID to signatures builders, sorted according to the order of /// transactions passed to the SignaturesBuilder. @@ -44,11 +44,11 @@ impl PetitionsStatus { impl Petitions { pub(crate) fn new( - factor_to_txid: HashMap>, + factor_source_to_intent_hash: HashMap>, txid_to_petition: IndexMap, ) -> Self { Self { - factor_to_txid, + factor_source_to_intent_hash, txid_to_petition: RefCell::new(txid_to_petition), } } @@ -57,21 +57,23 @@ impl Petitions { let txid_to_petition = self.txid_to_petition.into_inner(); let mut failed_transactions = MaybeSignedTransactions::empty(); let mut successful_transactions = MaybeSignedTransactions::empty(); - let mut skipped_factor_sources = IndexSet::<_>::new(); - for (txid, petition_of_transaction) in txid_to_petition.into_iter() { - let (successful, signatures, skipped) = petition_of_transaction.outcome(); - if successful { - successful_transactions.add_signatures(txid, signatures); + let mut neglected_factor_sources = IndexSet::::new(); + for (intent_hash, petition_of_transaction) in txid_to_petition.into_iter() { + let outcome = petition_of_transaction.outcome(); + let signatures = outcome.signatures; + + if outcome.transaction_valid { + successful_transactions.add_signatures(intent_hash, signatures); } else { - failed_transactions.add_signatures(txid, signatures); + failed_transactions.add_signatures(intent_hash, signatures); } - skipped_factor_sources.extend(skipped) + neglected_factor_sources.extend(outcome.neglected_factors) } SignaturesOutcome::new( successful_transactions, failed_transactions, - skipped_factor_sources, + neglected_factor_sources, ) } @@ -112,19 +114,23 @@ impl Petitions { PetitionsStatus::InProgressNoneInvalid } - pub fn invalid_transactions_if_skipped_factors( + pub fn invalid_transactions_if_neglected_factors( &self, factor_source_ids: IndexSet, - ) -> IndexSet { + ) -> IndexSet { factor_source_ids .clone() .iter() .flat_map(|f| { - self.factor_to_txid.get(f).unwrap().iter().flat_map(|txid| { - let binding = self.txid_to_petition.borrow(); - let value = binding.get(txid).unwrap(); - value.invalid_transactions_if_skipped_factors(factor_source_ids.clone()) - }) + self.factor_source_to_intent_hash + .get(f) + .unwrap() + .iter() + .flat_map(|intent_hash| { + let binding = self.txid_to_petition.borrow(); + let value = binding.get(intent_hash).unwrap(); + value.invalid_transactions_if_neglected_factors(factor_source_ids.clone()) + }) }) .collect::>() } @@ -133,12 +139,15 @@ impl Petitions { &self, factor_source_id: &FactorSourceIDFromHash, ) -> BatchTXBatchKeySigningRequest { - let txids = self.factor_to_txid.get(factor_source_id).unwrap(); - let per_transaction = txids + let intent_hashes = self + .factor_source_to_intent_hash + .get(factor_source_id) + .unwrap(); + let per_transaction = intent_hashes .into_iter() - .map(|txid| { + .map(|intent_hash| { let binding = self.txid_to_petition.borrow(); - let petition = binding.get(txid).unwrap(); + let petition = binding.get(intent_hash).unwrap(); petition.input_for_interactor(factor_source_id) }) .collect::>(); @@ -152,18 +161,21 @@ impl Petitions { petition.add_signature(signature.clone()) } - fn skip_factor_source_with_id(&self, skipped_factor_source_id: &FactorSourceIDFromHash) { + fn neglected_factor_source_with_id(&self, neglected: NeglectedFactor) { let binding = self.txid_to_petition.borrow(); - let txids = self.factor_to_txid.get(skipped_factor_source_id).unwrap(); - txids.into_iter().for_each(|txid| { - let petition = binding.get(txid).unwrap(); - petition.skipped_factor_source(skipped_factor_source_id) + let intent_hashes = self + .factor_source_to_intent_hash + .get(&neglected.factor_source_id()) + .unwrap(); + intent_hashes.into_iter().for_each(|intent_hash| { + let petition = binding.get(intent_hash).unwrap(); + petition.neglected_factor_source(neglected.clone()) }); } - pub(crate) fn process_batch_response(&self, response: SignWithFactorSourceOrSourcesOutcome) { + pub(crate) fn process_batch_response(&self, response: SignWithFactorsOutcome) { match response { - SignWithFactorSourceOrSourcesOutcome::Signed { + SignWithFactorsOutcome::Signed { produced_signatures, } => { for (k, v) in produced_signatures.signatures.clone().iter() { @@ -175,12 +187,14 @@ impl Petitions { .flatten() .for_each(|s| self.add_signature(s)); } - SignWithFactorSourceOrSourcesOutcome::Skipped { - ids_of_skipped_factors_sources, - } => { - for skipped_factor_source_id in ids_of_skipped_factors_sources.iter() { - info!("Skipped {}", skipped_factor_source_id); - self.skip_factor_source_with_id(skipped_factor_source_id) + SignWithFactorsOutcome::Neglected(neglected_factors) => { + let reason = neglected_factors.reason; + for neglected_factor_source_id in neglected_factors.content.iter() { + info!("Neglected {}", neglected_factor_source_id); + self.neglected_factor_source_with_id(NeglectedFactor::new( + reason, + *neglected_factor_source_id, + )) } } } @@ -239,6 +253,6 @@ mod tests { #[test] fn debug() { - pretty_assertions::assert_eq!(format!("{:?}", Sut::sample()), "Petitions(TXID(\"dedede\"): PetitionTransaction(for_entities: [PetitionEntity(intent_hash: TXID(\"dedede\"), entity: acco_Grace, \"threshold_factors PetitionFactors(input: PetitionFactorsInput(factors: {\\n factor_source_id: Device:de, derivation_path: 0/A/tx/0,\\n factor_source_id: Ledger:1e, derivation_path: 0/A/tx/1,\\n}), state_snapshot: signatures: \\\"\\\", skipped: \\\"\\\")\"\"override_factors PetitionFactors(input: PetitionFactorsInput(factors: {\\n factor_source_id: Ledger:1e, derivation_path: 0/A/tx/1,\\n}), state_snapshot: signatures: \\\"\\\", skipped: \\\"\\\")\")]))"); + pretty_assertions::assert_eq!(format!("{:?}", Sut::sample()), "Petitions(TXID(\"dedede\"): PetitionTransaction(for_entities: [PetitionEntity(intent_hash: TXID(\"dedede\"), entity: acco_Grace, \"threshold_factors PetitionFactors(input: PetitionFactorsInput(factors: {\\n factor_source_id: Device:de, derivation_path: 0/A/tx/0,\\n factor_source_id: Ledger:1e, derivation_path: 0/A/tx/1,\\n}), state_snapshot: signatures: \\\"\\\", neglected: \\\"\\\")\"\"override_factors PetitionFactors(input: PetitionFactorsInput(factors: {\\n factor_source_id: Ledger:1e, derivation_path: 0/A/tx/1,\\n}), state_snapshot: signatures: \\\"\\\", neglected: \\\"\\\")\")]))"); } } diff --git a/src/signing/signatures_outcome_types/signatures_outcome.rs b/src/signing/signatures_outcome_types/signatures_outcome.rs index 0a7a762d..ccf21df8 100644 --- a/src/signing/signatures_outcome_types/signatures_outcome.rs +++ b/src/signing/signatures_outcome_types/signatures_outcome.rs @@ -19,8 +19,9 @@ pub struct SignaturesOutcome { /// Potentially empty failed_transactions: MaybeSignedTransactions, - /// List of ids of all factor sources which failed. - skipped_factor_sources: IndexSet, + /// List of all neglected factor sources, either explicitly skipped by user or + /// implicitly neglected due to failure. + neglected_factor_sources: IndexSet, } impl SignaturesOutcome { @@ -30,14 +31,18 @@ impl SignaturesOutcome { pub fn new( successful_transactions: MaybeSignedTransactions, failed_transactions: MaybeSignedTransactions, - skipped_factor_sources: impl IntoIterator, + neglected_factor_sources: impl IntoIterator, ) -> Self { - let skipped_factor_sources = skipped_factor_sources.into_iter().collect::>(); + let neglected_factor_sources = neglected_factor_sources + .into_iter() + .collect::>(); + let successful_hashes: IndexSet = successful_transactions .transactions .keys() .cloned() .collect(); + let failure_hashes: IndexSet = failed_transactions.transactions.keys().cloned().collect(); @@ -54,7 +59,7 @@ impl SignaturesOutcome { Self { successful_transactions, failed_transactions, - skipped_factor_sources, + neglected_factor_sources, } } @@ -74,8 +79,15 @@ impl SignaturesOutcome { self.failed_transactions.clone().transactions() } - pub fn skipped_factor_sources(&self) -> IndexSet { - self.skipped_factor_sources.clone() + pub fn neglected_factor_sources(&self) -> IndexSet { + self.neglected_factor_sources.clone() + } + + pub fn ids_of_neglected_factor_sources(&self) -> IndexSet { + self.neglected_factor_sources() + .into_iter() + .map(|n| n.factor_source_id()) + .collect() } pub fn signatures_of_failed_transactions(&self) -> IndexSet { diff --git a/src/testing/signing/simulated_user.rs b/src/testing/signing/simulated_user.rs index 4b47b87c..4fca4617 100644 --- a/src/testing/signing/simulated_user.rs +++ b/src/testing/signing/simulated_user.rs @@ -122,7 +122,7 @@ pub enum Laziness { impl SimulatedUser { pub fn sign_or_skip( &self, - invalid_tx_if_skipped: impl IntoIterator, + invalid_tx_if_skipped: impl IntoIterator, ) -> SigningUserInput { let invalid_tx_if_skipped = invalid_tx_if_skipped .into_iter() diff --git a/src/testing/signing/test_interactors/test_parallel_interactor.rs b/src/testing/signing/test_interactors/test_parallel_interactor.rs index 7ae11923..bbe5a009 100644 --- a/src/testing/signing/test_interactors/test_parallel_interactor.rs +++ b/src/testing/signing/test_interactors/test_parallel_interactor.rs @@ -19,16 +19,20 @@ impl IsTestInteractor for TestSigningParallelInteractor { #[async_trait::async_trait] impl SignWithFactorParallelInteractor for TestSigningParallelInteractor { - async fn sign( - &self, - request: ParallelBatchSigningRequest, - ) -> Result { - if self.should_simulate_failure(request.per_factor_source.keys().cloned().collect()) { - return Err(CommonError::Failure); + async fn sign(&self, request: ParallelBatchSigningRequest) -> SignWithFactorsOutcome { + let ids = request + .per_factor_source + .keys() + .cloned() + .collect::>(); + + if self.should_simulate_failure(ids.clone()) { + return SignWithFactorsOutcome::user_skipped_factors(ids); } + match self .simulated_user - .sign_or_skip(request.invalid_transactions_if_skipped) + .sign_or_skip(request.invalid_transactions_if_neglected) { SigningUserInput::Sign => { let signatures = request @@ -57,16 +61,10 @@ impl SignWithFactorParallelInteractor for TestSigningParallelInteractor { .collect(), ); - Ok(SignWithFactorSourceOrSourcesOutcome::signed(response)) + SignWithFactorsOutcome::signed(response) } - SigningUserInput::Skip => Ok(SignWithFactorSourceOrSourcesOutcome::skipped( - request - .per_factor_source - .keys() - .cloned() - .collect::>(), - )), + SigningUserInput::Skip => SignWithFactorsOutcome::user_skipped_factors(ids), } } } diff --git a/src/testing/signing/test_interactors/test_serial_interactor.rs b/src/testing/signing/test_interactors/test_serial_interactor.rs index c532be27..5a74a7ac 100644 --- a/src/testing/signing/test_interactors/test_serial_interactor.rs +++ b/src/testing/signing/test_interactors/test_serial_interactor.rs @@ -19,17 +19,15 @@ impl IsTestInteractor for TestSigningSerialInteractor { #[async_trait::async_trait] impl SignWithFactorSerialInteractor for TestSigningSerialInteractor { - async fn sign( - &self, - request: SerialBatchSigningRequest, - ) -> Result { - if self.should_simulate_failure(IndexSet::from_iter([request.input.factor_source_id])) { - return Err(CommonError::Failure); + async fn sign(&self, request: SerialBatchSigningRequest) -> SignWithFactorsOutcome { + let ids = IndexSet::from_iter([request.input.factor_source_id]); + if self.should_simulate_failure(ids.clone()) { + return SignWithFactorsOutcome::failure_with_factors(ids); } - let invalid_transactions_if_skipped = request.invalid_transactions_if_skipped; + let invalid_transactions_if_neglected = request.invalid_transactions_if_neglected; match self .simulated_user - .sign_or_skip(invalid_transactions_if_skipped) + .sign_or_skip(invalid_transactions_if_neglected) { SigningUserInput::Sign => { let signatures = request @@ -52,12 +50,10 @@ impl SignWithFactorSerialInteractor for TestSigningSerialInteractor { .map(|(k, v)| (k, IndexSet::from_iter(v))) .collect(), ); - Ok(SignWithFactorSourceOrSourcesOutcome::signed(response)) + SignWithFactorsOutcome::signed(response) } SigningUserInput::Skip => { - Ok(SignWithFactorSourceOrSourcesOutcome::skipped_factor_source( - request.input.factor_source_id, - )) + SignWithFactorsOutcome::user_skipped_factor(request.input.factor_source_id) } } } diff --git a/src/types/factor_sources_of_kind.rs b/src/types/factor_sources_of_kind.rs index 2881395f..0ef7bcc8 100644 --- a/src/types/factor_sources_of_kind.rs +++ b/src/types/factor_sources_of_kind.rs @@ -30,13 +30,6 @@ impl FactorSourcesOfKind { pub(crate) fn factor_sources(&self) -> IndexSet { self.factor_sources.clone().into_iter().collect() } - - pub(crate) fn factor_source_ids(&self) -> Vec { - self.factor_sources - .iter() - .map(|f| f.factor_source_id()) - .collect() - } } #[cfg(test)] diff --git a/src/types/invalid_transaction_if_skipped.rs b/src/types/invalid_transaction_if_neglected.rs similarity index 85% rename from src/types/invalid_transaction_if_skipped.rs rename to src/types/invalid_transaction_if_neglected.rs index 043d4867..89628a18 100644 --- a/src/types/invalid_transaction_if_skipped.rs +++ b/src/types/invalid_transaction_if_neglected.rs @@ -1,19 +1,21 @@ use crate::prelude::*; /// A list of entities which would fail in a transaction if we would -/// skip signing with a certain factor source +/// neglect certain factor source, either by user explictlty skipping +/// it or if implicitly neglected due to failure. #[derive(Clone, Debug, PartialEq, Eq, std::hash::Hash)] -pub struct InvalidTransactionIfSkipped { - /// The intent hash of the transaction which would be invalid if we skipped - /// signing with a certain factor source +pub struct InvalidTransactionIfNeglected { + /// The intent hash of the transaction which would be invalid if a + /// certain factor source would be neglected, either if user + /// explicitly skipped it or implicitly neglected due to failure. pub intent_hash: IntentHash, /// The entities in the transaction which would fail auth. entities_which_would_fail_auth: Vec, } -impl InvalidTransactionIfSkipped { - /// Constructs a new `InvalidTransactionIfSkipped` from an IndexSet of +impl InvalidTransactionIfNeglected { + /// Constructs a new `InvalidTransactionIfNeglected` from an IndexSet of /// entities which would fail auth.. /// /// # Panics @@ -52,7 +54,7 @@ impl InvalidTransactionIfSkipped { #[cfg(test)] mod tests { use super::*; - type Sut = InvalidTransactionIfSkipped; + type Sut = InvalidTransactionIfNeglected; #[test] #[should_panic( diff --git a/src/types/mod.rs b/src/types/mod.rs index 03240900..b14ad4a1 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -1,7 +1,7 @@ mod factor_sources_of_kind; mod hd_signature; mod hd_signature_input; -mod invalid_transaction_if_skipped; +mod invalid_transaction_if_neglected; mod new_methods_on_sargon_types; mod owned_types; mod sargon_types; @@ -10,7 +10,7 @@ mod sign_with_factor_source_or_sources_outcome; pub(crate) use factor_sources_of_kind::*; pub use hd_signature::*; pub use hd_signature_input::*; -pub use invalid_transaction_if_skipped::*; +pub use invalid_transaction_if_neglected::*; pub use owned_types::*; pub use sargon_types::*; pub use sign_with_factor_source_or_sources_outcome::*; diff --git a/src/types/sign_with_factor_source_or_sources_outcome.rs b/src/types/sign_with_factor_source_or_sources_outcome.rs index cff4babb..fe9ab1a0 100644 --- a/src/types/sign_with_factor_source_or_sources_outcome.rs +++ b/src/types/sign_with_factor_source_or_sources_outcome.rs @@ -1,7 +1,7 @@ use crate::prelude::*; #[derive(Clone, PartialEq, Eq, derive_more::Debug)] -pub enum SignWithFactorSourceOrSourcesOutcome { +pub enum SignWithFactorsOutcome { /// The user successfully signed with the factor source(s), the associated /// value contains the produces signatures and any relevant metadata. #[debug("Signed: {:#?}", produced_signatures)] @@ -9,28 +9,35 @@ pub enum SignWithFactorSourceOrSourcesOutcome { produced_signatures: BatchSigningResponse, }, - /// The user skipped signing with the factor sources with ids - #[debug("Skipped")] - Skipped { - ids_of_skipped_factors_sources: Vec, - }, + /// The factor source got neglected, either due to user explicitly skipping + /// or due to failire + #[debug("Neglected")] + Neglected(NeglectedFactors), } -impl SignWithFactorSourceOrSourcesOutcome { +impl SignWithFactorsOutcome { pub fn signed(produced_signatures: BatchSigningResponse) -> Self { Self::Signed { produced_signatures, } } - pub fn skipped(ids_of_skipped_factors_sources: IndexSet) -> Self { - Self::Skipped { - ids_of_skipped_factors_sources: ids_of_skipped_factors_sources - .into_iter() - .collect_vec(), - } + pub fn failure_with_factors(ids: IndexSet) -> Self { + Self::Neglected(NeglectedFactors::new(NeglectFactorReason::Failure, ids)) + } + + pub fn user_skipped_factors(ids: IndexSet) -> Self { + Self::Neglected(NeglectedFactors::new( + NeglectFactorReason::UserExplicitlySkipped, + ids, + )) } - pub fn skipped_factor_source(factor_source_id: FactorSourceIDFromHash) -> Self { - Self::skipped(IndexSet::from_iter([factor_source_id])) + + pub fn user_skipped_factor(id: FactorSourceIDFromHash) -> Self { + Self::user_skipped_factors(IndexSet::from_iter([id])) + } + + pub fn failure_with_factor(id: FactorSourceIDFromHash) -> Self { + Self::failure_with_factors(IndexSet::from_iter([id])) } } diff --git a/tests/main.rs b/tests/main.rs index 20556628..4db58b1a 100644 --- a/tests/main.rs +++ b/tests/main.rs @@ -639,7 +639,7 @@ mod signing_tests { struct Expected { successful_txs_signature_count: usize, signed_factor_source_kinds: IndexSet, - expected_skipped_factor_source_count: usize, + expected_neglected_factor_source_count: usize, } async fn multi_securified_entities_with_sim_user(vector: Vector) { let factor_sources = &HDFactorSource::all(); @@ -677,8 +677,8 @@ mod signing_tests { let outcome = collector.collect_signatures().await; assert_eq!( - outcome.skipped_factor_sources().len(), - vector.expected.expected_skipped_factor_source_count + outcome.neglected_factor_sources().len(), + vector.expected.expected_neglected_factor_source_count ); assert!(outcome.successful()); @@ -730,7 +730,7 @@ mod signing_tests { FactorSourceKind::Arculus, FactorSourceKind::Yubikey, ]), - expected_skipped_factor_source_count: 1, + expected_neglected_factor_source_count: 1, }, }) .await; @@ -820,7 +820,7 @@ mod signing_tests { FactorSourceKind::Arculus, FactorSourceKind::Yubikey, ]), - expected_skipped_factor_source_count: 0, + expected_neglected_factor_source_count: 0, }, }) .await; @@ -836,7 +836,7 @@ mod signing_tests { FactorSourceKind::Yubikey, FactorSourceKind::Device, ]), - expected_skipped_factor_source_count: 2, + expected_neglected_factor_source_count: 2, }, }) .await; @@ -1101,7 +1101,7 @@ mod signing_tests { ); assert_eq!( - outcome.skipped_factor_sources(), + outcome.ids_of_neglected_factor_sources(), IndexSet::just(FactorSourceIDFromHash::fs1()) ) } @@ -1144,7 +1144,7 @@ mod signing_tests { assert!(signatures.is_empty()); } - async fn fail_get_skipped_e0() { + async fn fail_get_neglected_e0() { let failing = IndexSet::<_>::from_iter([FactorSourceIDFromHash::fs0()]); let collector = SignaturesCollector::test_prudent_with_failures( [TXToSign::new([E::e0()])], @@ -1152,8 +1152,8 @@ mod signing_tests { ); let outcome = collector.collect_signatures().await; assert!(!outcome.successful()); - let skipped = outcome.skipped_factor_sources(); - assert_eq!(skipped, failing); + let neglected = outcome.ids_of_neglected_factor_sources(); + assert_eq!(neglected, failing); } async fn lazy_always_skip_user_single_tx_e1() { @@ -1260,7 +1260,7 @@ mod signing_tests { let outcome = collector.collect_signatures().await; assert!(outcome.successful()); assert_eq!( - outcome.skipped_factor_sources(), + outcome.ids_of_neglected_factor_sources(), IndexSet::<_>::from_iter([FactorSourceIDFromHash::fs3()]) ); } @@ -1383,7 +1383,7 @@ mod signing_tests { #[actix_rt::test] async fn fail_get_skipped_a0() { - fail_get_skipped_e0::().await + fail_get_neglected_e0::().await } #[actix_rt::test] @@ -1558,7 +1558,7 @@ mod signing_tests { #[actix_rt::test] async fn fail_get_skipped_p0() { - fail_get_skipped_e0::().await + fail_get_neglected_e0::().await } #[actix_rt::test] From f0d4e848cf63f432017d4d0414204d42d6264cff Mon Sep 17 00:00:00 2001 From: Alexander Cyon Date: Wed, 28 Aug 2024 16:07:19 +0200 Subject: [PATCH 2/6] add _typos.toml --- _typos.toml | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 _typos.toml diff --git a/_typos.toml b/_typos.toml new file mode 100644 index 00000000..254d16c6 --- /dev/null +++ b/_typos.toml @@ -0,0 +1,3 @@ +default.extend-ignore-identifiers-re = [ + "[Hh][Dd][[:alnum:]]*" +] From c9e465d65440b4ec23ec1ccae985846c28339167 Mon Sep 17 00:00:00 2001 From: Alexander Cyon Date: Wed, 28 Aug 2024 16:10:23 +0200 Subject: [PATCH 3/6] naming --- src/signing/petition_types/petition_entity.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/signing/petition_types/petition_entity.rs b/src/signing/petition_types/petition_entity.rs index 471bcf7c..8ebf4ad4 100644 --- a/src/signing/petition_types/petition_entity.rs +++ b/src/signing/petition_types/petition_entity.rs @@ -94,12 +94,12 @@ impl PetitionEntity { .collect::>() } - pub fn all_neglected_factor_instance(&self) -> IndexSet { + pub fn all_neglected_factor_instances(&self) -> IndexSet { self.union_of(|f| f.all_neglected()) } pub fn all_neglected_factor_sources(&self) -> IndexSet { - self.all_neglected_factor_instance() + self.all_neglected_factor_instances() .into_iter() .map(|n| n.as_neglected_factor()) .collect::>() From 08783c63df6c18c42258ae3b270a4214d7e74eef Mon Sep 17 00:00:00 2001 From: Alexander Cyon Date: Wed, 28 Aug 2024 17:01:09 +0200 Subject: [PATCH 4/6] Documentation --- .../neglected_factor_instance.rs | 56 ++++++++++++------- 1 file changed, 36 insertions(+), 20 deletions(-) diff --git a/src/signing/petition_types/petition_factors_types/neglected_factor_instance.rs b/src/signing/petition_types/petition_factors_types/neglected_factor_instance.rs index b9ab2a55..991c8c4f 100644 --- a/src/signing/petition_types/petition_factors_types/neglected_factor_instance.rs +++ b/src/signing/petition_types/petition_factors_types/neglected_factor_instance.rs @@ -1,6 +1,33 @@ use crate::prelude::*; +/// A neglected factor, with a reason. +#[derive(Clone, PartialEq, Eq, Hash)] +pub struct AbstractNeglectedFactor { + /// The reason why this factor was neglected. + pub reason: NeglectFactorReason, + + /// The neglected factor + pub content: T, +} + +impl AbstractNeglectedFactor { + pub fn new(reason: NeglectFactorReason, content: T) -> Self { + Self { reason, content } + } +} + +impl std::fmt::Debug for AbstractNeglectedFactor { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Neglected") + .field("reason", &self.reason) + .field("content", &self.content) + .finish() + } +} + impl NeglectedFactorInstance { + /// Maps from `Neglected` + /// to `Neglected`, pub fn as_neglected_factor(&self) -> NeglectedFactor { NeglectedFactor::new(self.reason, self.factor_source_id()) } @@ -32,36 +59,25 @@ impl HasSampleValues for NeglectedFactorInstance { } } +/// ID to some neglected factor source, with the reason why it was neglected (skipped/failed) pub type NeglectedFactor = AbstractNeglectedFactor; -pub type NeglectedFactors = AbstractNeglectedFactor>; -pub type NeglectedFactorInstance = AbstractNeglectedFactor; -#[derive(Clone, PartialEq, Eq, Hash)] -pub struct AbstractNeglectedFactor { - pub reason: NeglectFactorReason, - pub content: T, -} -impl AbstractNeglectedFactor { - pub fn new(reason: NeglectFactorReason, content: T) -> Self { - Self { reason, content } - } -} +/// IDs to some neglected factor source, with the reason why they were neglected (skipped/failed) +pub type NeglectedFactors = AbstractNeglectedFactor>; -impl std::fmt::Debug for AbstractNeglectedFactor { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("Neglected") - .field("reason", &self.reason) - .field("content", &self.content) - .finish() - } -} +/// A HierarchicalDeterministicFactorInstance which was rejected, with the reason why (skipped/failed) +pub type NeglectedFactorInstance = AbstractNeglectedFactor; +/// Reason why some FactorSource was neglected, either explicitly skipped by the user +/// or implicitly neglected due to failure. #[derive(Clone, Copy, PartialEq, Eq, Hash, derive_more::Debug, derive_more::Display)] pub enum NeglectFactorReason { + /// A FactorSource got neglected since user explicitly skipped it. #[display("User Skipped")] #[debug("UserExplicitlySkipped")] UserExplicitlySkipped, + /// A FactorSource got neglected implicitly due to failure #[display("Failure")] #[debug("Failure")] Failure, From 1e5768d703ca7d720144d6d5b72a591096851193 Mon Sep 17 00:00:00 2001 From: Alexander Cyon Date: Thu, 29 Aug 2024 08:41:17 +0200 Subject: [PATCH 5/6] 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( ) { From 20fec9968fcff08b21c49bd4eb22870117a2132e Mon Sep 17 00:00:00 2001 From: Alexander Cyon Date: Thu, 29 Aug 2024 09:46:43 +0200 Subject: [PATCH 6/6] cleanup --- .../collector/key_derivation_outcome.rs | 4 ++ .../factor_source_referencing.rs | 2 +- .../petition_factors_types/mod.rs | 2 +- .../petition_types/petition_of_transaction.rs | 12 ++--- src/signing/signatures_outcome_types/mod.rs | 4 ++ .../petition_transaction_outcome.rs | 54 +++++++++++++++++++ .../sign_with_factors_outcome.rs} | 4 -- src/types/mod.rs | 2 - 8 files changed, 67 insertions(+), 17 deletions(-) create mode 100644 src/signing/signatures_outcome_types/petition_transaction_outcome.rs rename src/{types/sign_with_factor_source_or_sources_outcome.rs => signing/signatures_outcome_types/sign_with_factors_outcome.rs} (90%) diff --git a/src/derivation/collector/key_derivation_outcome.rs b/src/derivation/collector/key_derivation_outcome.rs index 57f95785..7a2d0400 100644 --- a/src/derivation/collector/key_derivation_outcome.rs +++ b/src/derivation/collector/key_derivation_outcome.rs @@ -1,10 +1,14 @@ use crate::prelude::*; +/// A collection of all `HierarchicalDeterministicFactorInstance` +/// (Public Keys) which were derived from the referenced +/// `FactorSource`s at the specified `DerivationPath`s #[derive(Debug, PartialEq, Eq, Clone)] pub struct KeyDerivationOutcome { pub factors_by_source: IndexMap>, } + impl KeyDerivationOutcome { pub fn new( factors_by_source: IndexMap< diff --git a/src/signing/petition_types/petition_factors_types/factor_source_referencing.rs b/src/signing/petition_types/petition_factors_types/factor_source_referencing.rs index 14ad5024..f4ab2087 100644 --- a/src/signing/petition_types/petition_factors_types/factor_source_referencing.rs +++ b/src/signing/petition_types/petition_factors_types/factor_source_referencing.rs @@ -1,6 +1,6 @@ use crate::prelude::*; -pub trait FactorSourceReferencing: std::hash::Hash + PartialEq + Eq + Clone { +pub(crate) trait FactorSourceReferencing: std::hash::Hash + PartialEq + Eq + Clone { fn factor_source_id(&self) -> FactorSourceIDFromHash; } diff --git a/src/signing/petition_types/petition_factors_types/mod.rs b/src/signing/petition_types/petition_factors_types/mod.rs index e8527b90..c5053a06 100644 --- a/src/signing/petition_types/petition_factors_types/mod.rs +++ b/src/signing/petition_types/petition_factors_types/mod.rs @@ -12,7 +12,7 @@ use petition_factors_state::*; use petition_factors_state_snapshot::*; use petition_factors_sub_state::*; -pub use factor_source_referencing::*; +pub(crate) use factor_source_referencing::*; pub use neglected_factor_instance::*; pub use petition_factors::*; pub use petition_factors_status::*; diff --git a/src/signing/petition_types/petition_of_transaction.rs b/src/signing/petition_types/petition_of_transaction.rs index 0f667b50..5a0b839a 100644 --- a/src/signing/petition_types/petition_of_transaction.rs +++ b/src/signing/petition_types/petition_of_transaction.rs @@ -11,13 +11,6 @@ pub(crate) struct PetitionTransaction { pub for_entities: RefCell>, } -#[derive(Clone, PartialEq, Eq)] -pub struct PetitionTransactionOutcome { - pub transaction_valid: bool, - pub signatures: IndexSet, - pub neglected_factors: IndexSet, -} - impl PetitionTransaction { pub(crate) fn new( intent_hash: IntentHash, @@ -62,11 +55,12 @@ impl PetitionTransaction { .flat_map(|x| x.all_neglected_factor_sources()) .collect::>(); - PetitionTransactionOutcome { + PetitionTransactionOutcome::new( transaction_valid, + self.intent_hash.clone(), signatures, neglected_factors, - } + ) } fn _all_factor_instances(&self) -> IndexSet { diff --git a/src/signing/signatures_outcome_types/mod.rs b/src/signing/signatures_outcome_types/mod.rs index 3c6324c4..7342ea65 100644 --- a/src/signing/signatures_outcome_types/mod.rs +++ b/src/signing/signatures_outcome_types/mod.rs @@ -1,5 +1,9 @@ mod maybe_signed_transactions; +mod petition_transaction_outcome; +mod sign_with_factors_outcome; mod signatures_outcome; pub use maybe_signed_transactions::*; +pub(crate) use petition_transaction_outcome::*; +pub use sign_with_factors_outcome::*; pub use signatures_outcome::*; diff --git a/src/signing/signatures_outcome_types/petition_transaction_outcome.rs b/src/signing/signatures_outcome_types/petition_transaction_outcome.rs new file mode 100644 index 00000000..72458fa2 --- /dev/null +++ b/src/signing/signatures_outcome_types/petition_transaction_outcome.rs @@ -0,0 +1,54 @@ +use crate::prelude::*; + +/// The outcome of collecting signatures for a specific +/// transasction - either valid or invalid - and a +/// set of collected signatues (might be empty) and +/// a set of neglected factors (might be empty). +#[derive(Clone, PartialEq, Eq)] +pub struct PetitionTransactionOutcome { + intent_hash: IntentHash, + pub transaction_valid: bool, + pub signatures: IndexSet, + pub neglected_factors: IndexSet, +} + +impl PetitionTransactionOutcome { + /// # Panics + /// Panics if the intent hash in any signatures does not + /// match `intent_hash` + pub fn new( + transaction_valid: bool, + intent_hash: IntentHash, + signatures: IndexSet, + neglected_factors: IndexSet, + ) -> Self { + assert!( + signatures.iter().all(|s| *s.intent_hash() == intent_hash), + "Discprenacy! Mismatching intent hash found in a signature." + ); + Self { + intent_hash, + transaction_valid, + signatures, + neglected_factors, + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + type Sut = PetitionTransactionOutcome; + + #[test] + #[should_panic(expected = "Discprenacy! Mismatching intent hash found in a signature.")] + fn panic() { + Sut::new( + true, + IntentHash::sample(), + IndexSet::from_iter([HDSignature::sample_other()]), + IndexSet::new(), + ); + } +} diff --git a/src/types/sign_with_factor_source_or_sources_outcome.rs b/src/signing/signatures_outcome_types/sign_with_factors_outcome.rs similarity index 90% rename from src/types/sign_with_factor_source_or_sources_outcome.rs rename to src/signing/signatures_outcome_types/sign_with_factors_outcome.rs index fe9ab1a0..776f0443 100644 --- a/src/types/sign_with_factor_source_or_sources_outcome.rs +++ b/src/signing/signatures_outcome_types/sign_with_factors_outcome.rs @@ -36,8 +36,4 @@ impl SignWithFactorsOutcome { pub fn user_skipped_factor(id: FactorSourceIDFromHash) -> Self { Self::user_skipped_factors(IndexSet::from_iter([id])) } - - pub fn failure_with_factor(id: FactorSourceIDFromHash) -> Self { - Self::failure_with_factors(IndexSet::from_iter([id])) - } } diff --git a/src/types/mod.rs b/src/types/mod.rs index b14ad4a1..740badb7 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -5,7 +5,6 @@ mod invalid_transaction_if_neglected; mod new_methods_on_sargon_types; mod owned_types; mod sargon_types; -mod sign_with_factor_source_or_sources_outcome; pub(crate) use factor_sources_of_kind::*; pub use hd_signature::*; @@ -13,4 +12,3 @@ pub use hd_signature_input::*; pub use invalid_transaction_if_neglected::*; pub use owned_types::*; pub use sargon_types::*; -pub use sign_with_factor_source_or_sources_outcome::*;