diff --git a/crates/rules/src/roles/abstract_role_builder_or_built.rs b/crates/rules/src/roles/abstract_role_builder_or_built.rs index 21e84cd4..ab7d57ed 100644 --- a/crates/rules/src/roles/abstract_role_builder_or_built.rs +++ b/crates/rules/src/roles/abstract_role_builder_or_built.rs @@ -18,7 +18,7 @@ pub struct AbstractRoleBuilderOrBuilt { pub type AbstractBuiltRoleWithFactor = AbstractRoleBuilderOrBuilt; pub type RoleBuilder = AbstractRoleBuilderOrBuilt; -impl AbstractBuiltRoleWithFactor { +impl AbstractRoleBuilderOrBuilt { pub(crate) fn with_factors( role: RoleKind, threshold: u8, diff --git a/crates/rules/src/roles/builder/roles_builder.rs b/crates/rules/src/roles/builder/roles_builder.rs index 7462547a..0f5db03c 100644 --- a/crates/rules/src/roles/builder/roles_builder.rs +++ b/crates/rules/src/roles/builder/roles_builder.rs @@ -245,11 +245,11 @@ impl RoleBuilder { } fn override_contains_factor_source(&self, factor_source_id: &FactorSourceID) -> bool { - self.override_factors().contains(&factor_source_id) + self.override_factors().contains(factor_source_id) } fn threshold_contains_factor_source(&self, factor_source_id: &FactorSourceID) -> bool { - self.threshold_factors().contains(&factor_source_id) + self.threshold_factors().contains(factor_source_id) } fn override_contains_factor_source_of_kind( @@ -257,7 +257,7 @@ impl RoleBuilder { factor_source_kind: FactorSourceKind, ) -> bool { self.override_factors() - .into_iter() + .iter() .any(|f| f.get_factor_source_kind() == factor_source_kind) } @@ -266,7 +266,7 @@ impl RoleBuilder { factor_source_kind: FactorSourceKind, ) -> bool { self.threshold_factors() - .into_iter() + .iter() .any(|f| f.get_factor_source_kind() == factor_source_kind) } @@ -319,15 +319,27 @@ impl RoleBuilder { } // Validate threshold count - if self.threshold_factors().len() < self.threshold() as usize { - return RoleBuilderMutateResult::not_yet_valid( - NotYetValidReason::ThresholdHigherThanThresholdFactorsLen, - ); - } - if self.threshold() == 0 && !self.threshold_factors().is_empty() { - return RoleBuilderMutateResult::not_yet_valid( - NotYetValidReason::PrimaryRoleWithThresholdCannotBeZeroWithFactors, - ); + if self.role() == RoleKind::Primary { + if self.threshold_factors().len() < self.threshold() as usize { + return RoleBuilderMutateResult::not_yet_valid( + NotYetValidReason::ThresholdHigherThanThresholdFactorsLen, + ); + } + if self.threshold() == 0 && !self.threshold_factors().is_empty() { + return RoleBuilderMutateResult::not_yet_valid( + NotYetValidReason::PrimaryRoleWithThresholdCannotBeZeroWithFactors, + ); + } + } else if self.threshold() != 0 { + match self.role() { + Primary => unreachable!("Primary role should have been handled earlier"), + Recovery => { + return RoleBuilderMutateResult::basic_violation(RecoveryCannotSetThreshold) + } + Confirmation => { + return RoleBuilderMutateResult::basic_violation(ConfirmationCannotSetThreshold) + } + } } if self.factors().is_empty() { @@ -448,10 +460,10 @@ impl RoleBuilder { }; if self.override_contains_factor_source(factor_source_id) { - remove(&mut self.mut_override_factors()) + remove(self.mut_override_factors()) } if self.threshold_contains_factor_source(factor_source_id) { - remove(&mut self.mut_threshold_factors()); + remove(self.mut_threshold_factors()); let threshold_factors_len = self.threshold_factors().len() as u8; if self.threshold() > threshold_factors_len { self.set_threshold(threshold_factors_len)?; @@ -584,13 +596,12 @@ impl RoleBuilder { .collect() }; match factor_list_kind { - FactorListKind::Override => filter(&self.override_factors()), - FactorListKind::Threshold => filter(&self.threshold_factors()), + FactorListKind::Override => filter(self.override_factors()), + FactorListKind::Threshold => filter(self.threshold_factors()), } } } - #[cfg(test)] mod tests { @@ -769,6 +780,87 @@ mod tests { }); } + #[test] + fn validate_override_for_ever_invalid() { + let sut = SUT::with_factors( + RoleKind::Primary, + 0, + vec![], + vec![ + FactorSourceID::sample_ledger(), + FactorSourceID::sample_ledger(), + ], + ); + let res = sut.validate(); + assert_eq!( + res, + MutRes::forever_invalid(ForeverInvalidReason::FactorSourceAlreadyPresent) + ); + } + + #[test] + fn validate_threshold_for_ever_invalid() { + let sut = SUT::with_factors( + RoleKind::Primary, + 1, + vec![ + FactorSourceID::sample_ledger(), + FactorSourceID::sample_ledger(), + ], + vec![], + ); + let res = sut.validate(); + assert_eq!( + res, + MutRes::forever_invalid(ForeverInvalidReason::FactorSourceAlreadyPresent) + ); + } + + #[test] + fn confirmation_validate_basic_violation() { + let sut = SUT::with_factors( + RoleKind::Confirmation, + 1, + vec![], + vec![FactorSourceID::sample_ledger()], + ); + let res = sut.validate(); + assert_eq!( + res, + MutRes::basic_violation(BasicViolation::ConfirmationCannotSetThreshold) + ); + } + + #[test] + fn recovery_validate_basic_violation() { + let sut = SUT::with_factors( + RoleKind::Recovery, + 1, + vec![], + vec![FactorSourceID::sample_ledger()], + ); + let res = sut.validate(); + assert_eq!( + res, + MutRes::basic_violation(BasicViolation::RecoveryCannotSetThreshold) + ); + } + + #[test] + fn primary_validate_not_yet_valid_for_threshold_greater_than_threshold_factors() { + let sut = SUT::with_factors( + RoleKind::Primary, + 1, + vec![], + vec![FactorSourceID::sample_ledger()], + ); + let res = sut.validate(); + assert_eq!( + res, + MutRes::not_yet_valid(ThresholdHigherThanThresholdFactorsLen) + ); + } + #[cfg(test)] mod recovery_in_isolation {