diff --git a/rs/sns/swap/src/swap.rs b/rs/sns/swap/src/swap.rs index 5fb168b74f4..0fbdbfb7d29 100644 --- a/rs/sns/swap/src/swap.rs +++ b/rs/sns/swap/src/swap.rs @@ -559,7 +559,7 @@ impl Swap { direct_participation_icp_e8s: None, neurons_fund_participation_icp_e8s: None, }; - if init.is_swap_init_for_one_proposal_flow() { + if init.validate_swap_init_for_one_proposal_flow().is_ok() { // Automatically fill out the fields that the (legacy) open request // used to provide, supporting clients who read legacy Swap fields. { diff --git a/rs/sns/swap/src/types.rs b/rs/sns/swap/src/types.rs index f1780ff3aef..de97cfe83a7 100644 --- a/rs/sns/swap/src/types.rs +++ b/rs/sns/swap/src/types.rs @@ -406,7 +406,7 @@ impl Init { /// - `self.is_swap_init_for_single_proposal()` pub fn mk_open_sns_request(&self) -> OpenRequest { assert!( - self.is_swap_init_for_one_proposal_flow(), + self.validate_swap_init_for_one_proposal_flow().is_ok(), "cannot make an `OpenRequest` instance from a legacy `SnsInitPayload`" ); // This is checked by the previous assert, but repeating here to be explicit @@ -447,19 +447,53 @@ impl Init { /// Indicates whether this swap `Init` payload matches the legacy structure, /// i.e., all of its swap-opening fields (see `swap_opening_field_states`) /// are **unset**, as they will be passed explicitly via the `Swap.open` API. - pub fn is_swap_init_for_legacy(&self) -> bool { - self.swap_opening_field_states(None) - .values() - .all(|x| x.is_none()) + pub fn validate_swap_init_for_legacy(&self) -> Result<(), String> { + let unexpected_fields: Vec = self + .swap_opening_field_states(None) + .iter() + .filter_map(|(field_name, outcome)| { + if outcome.is_some() { + Some(field_name.clone()) + } else { + None + } + }) + .collect(); + if unexpected_fields.is_empty() { + Ok(()) + } else { + Err(format!( + "Swap Init cannot be used for legacy flow as there are {} unexpected fields: {}.", + unexpected_fields.len(), + unexpected_fields.join(", "), + )) + } } /// Indicates whether this swap `Init` payload matches the new structure, /// i.e., all of its swap-opening fields (see `swap_opening_field_states`) /// are **set**. - pub fn is_swap_init_for_one_proposal_flow(&self) -> bool { - self.swap_opening_field_states(None) - .values() - .all(|x| x.is_some()) + pub fn validate_swap_init_for_one_proposal_flow(&self) -> Result<(), String> { + let missing_fields: Vec = self + .swap_opening_field_states(None) + .iter() + .filter_map(|(field_name, outcome)| { + if outcome.is_none() { + Some(field_name.clone()) + } else { + None + } + }) + .collect(); + if missing_fields.is_empty() { + Ok(()) + } else { + Err(format!( + "Swap Init cannot be used for 1-proposal as there are {} missing fields: {}.", + missing_fields.len(), + missing_fields.join(", "), + )) + } } /// Indicates whether an `OpenRequest` instance contradicts this swap `Init` @@ -503,11 +537,14 @@ impl Init { // TODO[NNS1-2362] Re-validate also the fields that were filled out by // TODO[NNS1-2362] trusted code. - - if !self.is_swap_init_for_legacy() && !self.is_swap_init_for_one_proposal_flow() { - return Err( - "fields listed in `swap_opening_field_states` must either all be set (for 1-proposal) or all be unset (legacy).".to_string() - ); + if let (Err(legacy_error), Err(one_proposal_error)) = ( + self.validate_swap_init_for_legacy(), + self.validate_swap_init_for_one_proposal_flow(), + ) { + return Err(format!( + "Swap Init is inconsistent. On the one hand, {}. But on the other hand, {}.", + legacy_error, one_proposal_error, + )); } if self.should_auto_finalize.is_none() { @@ -1613,7 +1650,7 @@ mod tests { // fields are `None`). { let default_init: Init = Default::default(); - assert!(default_init.is_swap_init_for_legacy()); + assert!(default_init.validate_swap_init_for_legacy().is_ok()); } // There exists some `SnsInitPayload` that is not suitable for both // single-proposal and legacy flows. @@ -1623,8 +1660,10 @@ mod tests { ..Default::default() }; assert!( - !incorrect_init.is_swap_init_for_one_proposal_flow() - && !incorrect_init.is_swap_init_for_legacy() + incorrect_init + .validate_swap_init_for_one_proposal_flow() + .is_err() + && incorrect_init.validate_swap_init_for_legacy().is_err() ); } let mut init = Init { @@ -1650,7 +1689,7 @@ mod tests { }; // There exists some `SnsInitPayload` that is suitable for the new // single-proposal flow. - assert!(init.is_swap_init_for_one_proposal_flow()); + assert!(init.validate_swap_init_for_one_proposal_flow().is_ok()); let neurons_fund_participant_a = CfParticipant { hotkey_principal: "HotKeyA".to_string(),