Merge pull request #5 from radixdlt/dynamic_factors

Dynamic factors

.pre-commit-config.yaml

@@ -2,7 +2,7 @@ fail_fast: true
 
 repos:
   - repo: https://github.com/crate-ci/typos
-    rev: v1.22.9
+    rev: v1.24.1
     hooks:
       - id: typos
   - repo: local

src/signing/collector/signatures_collector.rs

@@ -144,7 +144,38 @@ impl SignaturesCollector {
         Continue
     }
 
-    async fn sign_with_factors_of_kind(&self, factor_sources_of_kind: FactorSourcesOfKind) {
+    fn should_neglect_factors_due_to_irrelevant(
+        &self,
+        factor_sources_of_kind: &FactorSourcesOfKind,
+    ) -> bool {
+        let state = self.state.borrow();
+        let petitions = state.petitions.borrow();
+        petitions.should_neglect_factors_due_to_irrelevant(factor_sources_of_kind)
+    }
+
+    fn neglected_factors_due_to_irrelevant(
+        &self,
+        factor_sources_of_kind: &FactorSourcesOfKind,
+    ) -> bool {
+        if self.should_neglect_factors_due_to_irrelevant(factor_sources_of_kind) {
+            info!(
+                "Neglecting all factors of kind: {} since they are all irrelevant (all TX referencing those factors have already failed)",
+                factor_sources_of_kind.kind
+            );
+            self.process_batch_response(SignWithFactorsOutcome::irrelevant(factor_sources_of_kind));
+            true
+        } else {
+            false
+        }
+    }
+
+    async fn sign_with_factors_of_kind(&self, factor_sources_of_kind: &FactorSourcesOfKind) {
+        info!(
+            "Use(?) #{:?} factors of kind: {:?}",
+            &factor_sources_of_kind.factor_sources().len(),
+            &factor_sources_of_kind.kind
+        );
+
         let interactor = self
             .dependencies
             .interactors
@@ -155,12 +186,7 @@ impl SignaturesCollector {
             SigningInteractor::Parallel(interactor) => {
                 // Prepare the request for the interactor
                 debug!("Creating parallel request for interactor");
-                let request = self.request_for_parallel_interactor(
-                    factor_sources
-                        .into_iter()
-                        .map(|f| f.factor_source_id())
-                        .collect(),
-                );
+                let request = self.request_for_parallel_interactor(factor_sources_of_kind);
                 if !request.invalid_transactions_if_neglected.is_empty() {
                     info!(
                         "If factors {:?} are neglected, invalid TXs: {:?}",
@@ -212,15 +238,13 @@ impl SignaturesCollector {
     /// In decreasing "friction order"
     async fn sign_with_factors(&self) -> Result<()> {
         let factors_of_kind = self.dependencies.factors_of_kind.clone();
-        for factor_sources_of_kind in factors_of_kind.into_iter() {
+        for factor_sources_of_kind in factors_of_kind.iter() {
             if self.continuation() == FinishEarly {
                 break;
             }
-            info!(
-                "Use(?) #{:?} factors of kind: {:?}",
-                &factor_sources_of_kind.factor_sources().len(),
-                &factor_sources_of_kind.kind
-            );
+            if self.neglected_factors_due_to_irrelevant(factor_sources_of_kind) {
+                continue;
+            }
             self.sign_with_factors_of_kind(factor_sources_of_kind).await;
         }
         info!("FINISHED WITH ALL FACTORS");
@@ -250,14 +274,19 @@ impl SignaturesCollector {
                 *factor_source_id,
             ]))
             .into_iter()
-            .collect_vec(),
+            .collect::<IndexSet<_>>(),
         )
     }
 
     fn request_for_parallel_interactor(
         &self,
-        factor_source_ids: IndexSet<FactorSourceIDFromHash>,
+        factor_sources_of_kind: &FactorSourcesOfKind,
     ) -> ParallelBatchSigningRequest {
+        let factor_source_ids = factor_sources_of_kind
+            .factor_sources()
+            .iter()
+            .map(|f| f.factor_source_id())
+            .collect::<IndexSet<_>>();
         let per_factor_source = factor_source_ids
             .clone()
             .iter()
@@ -267,13 +296,12 @@ impl SignaturesCollector {
         let invalid_transactions_if_neglected =
             self.invalid_transactions_if_neglected_factor_sources(factor_source_ids);
 
-        info!(
-            "Invalid if neglected: {:?}",
-            invalid_transactions_if_neglected
-        );
-
         // Prepare the request for the interactor
-        ParallelBatchSigningRequest::new(per_factor_source, invalid_transactions_if_neglected)
+        ParallelBatchSigningRequest::new(
+            factor_sources_of_kind.kind,
+            per_factor_source,
+            invalid_transactions_if_neglected,
+        )
     }
 
     fn invalid_transactions_if_neglected_factor_sources(
@@ -441,6 +469,28 @@ mod tests {
         test(WhenAllTransactionsAreValid(Continue), 2).await;
     }
 
+    #[test]
+    fn factor_source_kinds_order() {
+        let kinds = HDFactorSource::all()
+            .into_iter()
+            .map(|f| f.factor_source_kind())
+            .collect::<IndexSet<_>>();
+        let mut kinds = kinds.into_iter().collect_vec();
+        kinds.sort();
+        let kinds = kinds.into_iter().collect::<IndexSet<_>>();
+        assert_eq!(
+            kinds,
+            IndexSet::<FactorSourceKind>::from_iter([
+                FactorSourceKind::Ledger,
+                FactorSourceKind::Arculus,
+                FactorSourceKind::Yubikey,
+                FactorSourceKind::SecurityQuestions,
+                FactorSourceKind::OffDeviceMnemonic,
+                FactorSourceKind::Device,
+            ])
+        )
+    }
+
     #[test]
     fn test_profile() {
         let factor_sources = &HDFactorSource::all();

src/signing/collector/signing_finish_early_strategy.rs

@@ -14,7 +14,7 @@ pub struct WhenSomeTransactionIsInvalid(pub SignaturesCollectingContinuation);
 
 impl Default for WhenSomeTransactionIsInvalid {
     fn default() -> Self {
-        Self(SignaturesCollectingContinuation::FinishEarly)
+        Self(SignaturesCollectingContinuation::Continue)
     }
 }
 
@@ -60,4 +60,17 @@ mod tests {
             SignaturesCollectingContinuation::Continue
         );
     }
+
+    #[test]
+    fn test_default_is_finish_when_valid_continue_if_invalid() {
+        let sut = Sut::default();
+        assert_eq!(
+            sut.when_all_transactions_are_valid.0,
+            SignaturesCollectingContinuation::FinishEarly
+        );
+        assert_eq!(
+            sut.when_some_transaction_is_invalid.0,
+            SignaturesCollectingContinuation::Continue
+        );
+    }
 }

src/signing/interactors/batch_tx_batch_key_signing_request.rs

@@ -17,28 +17,94 @@ pub struct BatchKeySigningRequest {
 }
 
 impl BatchKeySigningRequest {
-    pub fn signature_inputs(&self) -> IndexSet<HDSignatureInput> {
-        self.owned_factor_instances
-            .clone()
-            .into_iter()
-            .map(|fi| HDSignatureInput::new(self.intent_hash.clone(), fi))
-            .collect()
-    }
-
+    /// # Panics
+    /// Panics if any of the owned factor instances does not match the `factor_source_id`.
+    ///
+    /// Panics if `owned_factor_instances` is empty.
     pub fn new(
         intent_hash: IntentHash,
         factor_source_id: FactorSourceIDFromHash,
         owned_factor_instances: IndexSet<OwnedFactorInstance>,
     ) -> Self {
+        assert!(
+            !owned_factor_instances.is_empty(),
+            "Invalid input, `owned_factor_instances` must not be empty."
+        );
         assert!(owned_factor_instances
             .iter()
-            .all(|f| f.by_factor_source(factor_source_id)));
+            .all(|f| f.by_factor_source(factor_source_id)), "Discrepancy! Mismatch between FactorSourceID of owned factor instances and specified FactorSourceID, this is a programmer error.");
         Self {
             intent_hash,
             factor_source_id,
             owned_factor_instances: owned_factor_instances.into_iter().collect_vec(),
         }
     }
+
+    pub fn signature_inputs(&self) -> IndexSet<HDSignatureInput> {
+        self.owned_factor_instances
+            .clone()
+            .into_iter()
+            .map(|fi| HDSignatureInput::new(self.intent_hash.clone(), fi))
+            .collect()
+    }
+}
+
+impl HasSampleValues for BatchKeySigningRequest {
+    fn sample() -> Self {
+        Self::new(
+            IntentHash::sample(),
+            FactorSourceIDFromHash::sample(),
+            IndexSet::from_iter([OwnedFactorInstance::sample()]),
+        )
+    }
+
+    fn sample_other() -> Self {
+        Self::new(
+            IntentHash::sample_other(),
+            FactorSourceIDFromHash::sample_other(),
+            IndexSet::from_iter([OwnedFactorInstance::sample_other()]),
+        )
+    }
+}
+
+#[cfg(test)]
+mod tests_batch_req {
+    use super::*;
+
+    type Sut = BatchKeySigningRequest;
+
+    #[test]
+    fn equality() {
+        assert_eq!(Sut::sample(), Sut::sample());
+        assert_eq!(Sut::sample_other(), Sut::sample_other());
+    }
+
+    #[test]
+    fn inequality() {
+        assert_ne!(Sut::sample(), Sut::sample_other());
+    }
+
+    #[test]
+    #[should_panic(expected = "Invalid input, `owned_factor_instances` must not be empty.")]
+    fn panics_if_owned_factors_is_empty() {
+        Sut::new(
+            IntentHash::sample(),
+            FactorSourceIDFromHash::sample(),
+            IndexSet::new(),
+        );
+    }
+
+    #[test]
+    #[should_panic(
+        expected = "Discrepancy! Mismatch between FactorSourceID of owned factor instances and specified FactorSourceID, this is a programmer error."
+    )]
+    fn panics_mismatch_factor_source_id() {
+        Sut::new(
+            IntentHash::sample(),
+            FactorSourceIDFromHash::sample(),
+            IndexSet::from_iter([OwnedFactorInstance::sample_other()]),
+        );
+    }
 }
 
 /// A batch of transactions each batching over multiple keys (derivation paths)
@@ -53,16 +119,81 @@ pub struct BatchTXBatchKeySigningRequest {
 }
 
 impl BatchTXBatchKeySigningRequest {
+    /// # Panics
+    /// Panics if `per_transaction` is empty
+    ///
+    /// Also panics if `per_transaction` if the factor source id
+    /// of each request does not match `factor_source_id`.
     pub fn new(
         factor_source_id: FactorSourceIDFromHash,
         per_transaction: IndexSet<BatchKeySigningRequest>,
     ) -> Self {
+        assert!(
+            !per_transaction.is_empty(),
+            "Invalid input. No transaction to sign, this is a programmer error."
+        );
+
         assert!(per_transaction
             .iter()
-            .all(|f| f.factor_source_id == factor_source_id));
+            .all(|f| f.factor_source_id == factor_source_id), "Discprepancy! Input for one of the transactions has a mismatching FactorSourceID, this is a programmer error.");
+
         Self {
             factor_source_id,
             per_transaction: per_transaction.into_iter().collect(),
         }
     }
+
+    pub fn factor_source_kind(&self) -> FactorSourceKind {
+        self.factor_source_id.kind
+    }
+}
+
+impl HasSampleValues for BatchTXBatchKeySigningRequest {
+    fn sample() -> Self {
+        Self::new(
+            FactorSourceIDFromHash::sample(),
+            IndexSet::from_iter([BatchKeySigningRequest::sample()]),
+        )
+    }
+
+    fn sample_other() -> Self {
+        Self::new(
+            FactorSourceIDFromHash::sample_other(),
+            IndexSet::from_iter([BatchKeySigningRequest::sample_other()]),
+        )
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    type Sut = BatchTXBatchKeySigningRequest;
+
+    #[test]
+    fn equality() {
+        assert_eq!(Sut::sample(), Sut::sample());
+        assert_eq!(Sut::sample_other(), Sut::sample_other());
+    }
+
+    #[test]
+    fn inequality() {
+        assert_ne!(Sut::sample(), Sut::sample_other());
+    }
+
+    #[test]
+    #[should_panic(expected = "Invalid input. No transaction to sign, this is a programmer error.")]
+    fn panics_if_per_transaction_is_empty() {
+        Sut::new(FactorSourceIDFromHash::sample(), IndexSet::new());
+    }
+
+    #[test]
+    #[should_panic(
+        expected = "Discprepancy! Input for one of the transactions has a mismatching FactorSourceID, this is a programmer error."
+    )]
+    fn panics_if_factor_source_mismatch() {
+        Sut::new(
+            FactorSourceIDFromHash::sample(),
+            IndexSet::from_iter([BatchKeySigningRequest::sample_other()]),
+        );
+    }
 }