fix: paid/unpaid classification for invalid batch transitions

packages/rs-dpp/src/validation/validation_result.rs

@@ -34,6 +34,22 @@ impl<T: Clone, E: Debug> Default for ValidationResult<T, E> {
 }
 
 impl<TData: Clone, E: Debug> ValidationResult<Vec<TData>, E> {
+    /// **Deprecated.** Always returns `data: Some(Vec<...>)` — even if no
+    /// input contributed any data — which violates the implicit contract
+    /// `data.is_none() ⇔ no work done` that downstream `process_validation_result`
+    /// keys on. See issue #2867 (the empty-action / "validating state
+    /// transition for free" bug). Use [`flatten_strict`] instead, which
+    /// returns `data: None` when no input contributed data.
+    ///
+    /// Preserved for `PROTOCOL_VERSION_11` and below — changing this
+    /// function's behavior would be a consensus-breaking change for the
+    /// existing chain history.
+    ///
+    /// [`flatten_strict`]: ValidationResult::flatten_strict
+    #[deprecated(
+        since = "3.1.0",
+        note = "use flatten_strict; flatten always returns Some(empty_vec) which violates the data-is-None ⇔ no-work invariant — see issue #2867"
+    )]
     pub fn flatten<I: IntoIterator<Item = ValidationResult<Vec<TData>, E>>>(
         items: I,
     ) -> ValidationResult<Vec<TData>, E> {
@@ -48,9 +64,58 @@ impl<TData: Clone, E: Debug> ValidationResult<Vec<TData>, E> {
         });
         ValidationResult::new_with_data_and_errors(aggregate_data, aggregate_errors)
     }
+
+    /// Strict variant of [`flatten`]: returns `data: None` when no input
+    /// contributed any data (i.e. every input was either `data: None` or
+    /// `data: Some(empty_vec)`), and only returns `data: Some(...)` when
+    /// the aggregate Vec is non-empty.
+    ///
+    /// This restores the invariant that `data.is_none() ⇔ no work done`,
+    /// which downstream code (e.g.
+    /// `process_validation_result_v0:241`) relies on to choose between
+    /// `PaidConsensusError` and `UnpaidConsensusError`. Used by
+    /// `PROTOCOL_VERSION_12`+ to close the issue #2867 "validating state
+    /// transition for free" gap.
+    ///
+    /// [`flatten`]: ValidationResult::flatten
+    pub fn flatten_strict<I: IntoIterator<Item = ValidationResult<Vec<TData>, E>>>(
+        items: I,
+    ) -> ValidationResult<Vec<TData>, E> {
+        let mut aggregate_errors = vec![];
+        let mut aggregate_data = vec![];
+        items.into_iter().for_each(|single_validation_result| {
+            let ValidationResult { mut errors, data } = single_validation_result;
+            aggregate_errors.append(&mut errors);
+            if let Some(mut data) = data {
+                aggregate_data.append(&mut data);
+            }
+        });
+        if aggregate_data.is_empty() {
+            ValidationResult {
+                errors: aggregate_errors,
+                data: None,
+            }
+        } else {
+            ValidationResult::new_with_data_and_errors(aggregate_data, aggregate_errors)
+        }
+    }
 }
 
 impl<TData: Clone, E: Debug> ValidationResult<TData, E> {
+    /// **Deprecated.** Always returns `data: Some(Vec<...>)` — even if no
+    /// input contributed any data — which violates the implicit contract
+    /// `data.is_none() ⇔ no work done`. See issue #2867. Use
+    /// [`merge_many_strict`] instead.
+    ///
+    /// Preserved for `PROTOCOL_VERSION_11` and below — changing this
+    /// function's behavior would be a consensus-breaking change for the
+    /// existing chain history.
+    ///
+    /// [`merge_many_strict`]: ValidationResult::merge_many_strict
+    #[deprecated(
+        since = "3.1.0",
+        note = "use merge_many_strict; merge_many always returns Some(empty_vec) which violates the data-is-None ⇔ no-work invariant — see issue #2867"
+    )]
     pub fn merge_many<I: IntoIterator<Item = ValidationResult<TData, E>>>(
         items: I,
     ) -> ValidationResult<Vec<TData>, E> {
@@ -65,6 +130,37 @@ impl<TData: Clone, E: Debug> ValidationResult<TData, E> {
         });
         ValidationResult::new_with_data_and_errors(aggregate_data, aggregate_errors)
     }
+
+    /// Strict variant of [`merge_many`]: returns `data: None` when no
+    /// input had `Some(data)`, and only returns `data: Some(Vec<...>)`
+    /// when at least one input contributed data.
+    ///
+    /// This restores the `data.is_none() ⇔ no work done` invariant — see
+    /// issue #2867. Used by `PROTOCOL_VERSION_12`+ to close the
+    /// "validating state transition for free" gap.
+    ///
+    /// [`merge_many`]: ValidationResult::merge_many
+    pub fn merge_many_strict<I: IntoIterator<Item = ValidationResult<TData, E>>>(
+        items: I,
+    ) -> ValidationResult<Vec<TData>, E> {
+        let mut aggregate_errors = vec![];
+        let mut aggregate_data = vec![];
+        items.into_iter().for_each(|single_validation_result| {
+            let ValidationResult { mut errors, data } = single_validation_result;
+            aggregate_errors.append(&mut errors);
+            if let Some(data) = data {
+                aggregate_data.push(data);
+            }
+        });
+        if aggregate_data.is_empty() {
+            ValidationResult {
+                errors: aggregate_errors,
+                data: None,
+            }
+        } else {
+            ValidationResult::new_with_data_and_errors(aggregate_data, aggregate_errors)
+        }
+    }
 }
 
 impl<E: Debug> SimpleValidationResult<E> {
@@ -539,9 +635,12 @@ mod tests {
         assert_eq!(result.errors, vec!["bad".to_string()]);
     }
 
-    // -- flatten() --
+    // -- flatten() (deprecated) --
+    // These pin the historical buggy behavior preserved for
+    // PROTOCOL_VERSION_11 and below — issue #2867.
 
     #[test]
+    #[allow(deprecated)]
     fn test_flatten_merges_data_and_errors() {
         let r1: ValidationResult<Vec<i32>, String> = ValidationResult::new_with_data(vec![1, 2]);
         let r2: ValidationResult<Vec<i32>, String> =
@@ -555,16 +654,36 @@ mod tests {
     }
 
     #[test]
+    #[allow(deprecated)]
     fn test_flatten_empty_input() {
         let flat: ValidationResult<Vec<i32>, String> =
             ValidationResult::flatten(std::iter::empty());
+        // Issue #2867 root cause: flatten produces Some(empty_vec) here,
+        // not None. Downstream code that checks `data.is_none()` is fooled
+        // into treating "no data" as "has data".
         assert_eq!(flat.data, Some(vec![]));
         assert!(flat.errors.is_empty());
     }
 
-    // -- merge_many() --
+    #[test]
+    #[allow(deprecated)]
+    fn test_flatten_all_inputs_no_data_returns_some_empty() {
+        // Pins the buggy v11 behavior: all inputs have data:None, but
+        // flatten still produces data:Some(vec![]).
+        let r1: ValidationResult<Vec<i32>, String> =
+            ValidationResult::new_with_error("e1".to_string());
+        let r2: ValidationResult<Vec<i32>, String> =
+            ValidationResult::new_with_error("e2".to_string());
+
+        let flat = ValidationResult::flatten(vec![r1, r2]);
+        assert_eq!(flat.data, Some(vec![]));
+        assert_eq!(flat.errors, vec!["e1".to_string(), "e2".to_string()]);
+    }
+
+    // -- merge_many() (deprecated) --
 
     #[test]
+    #[allow(deprecated)]
     fn test_merge_many_collects_data_into_vec() {
         let r1: ValidationResult<i32, String> = ValidationResult::new_with_data(1);
         let r2: ValidationResult<i32, String> = ValidationResult::new_with_data(2);
@@ -576,13 +695,135 @@ mod tests {
     }
 
     #[test]
+    #[allow(deprecated)]
     fn test_merge_many_empty_input() {
         let merged: ValidationResult<Vec<i32>, String> =
             ValidationResult::merge_many(std::iter::empty::<ValidationResult<i32, String>>());
+        // Same buggy shape: Some(empty_vec) instead of None.
+        assert_eq!(merged.data, Some(vec![]));
+        assert!(merged.errors.is_empty());
+    }
+
+    #[test]
+    #[allow(deprecated)]
+    fn test_merge_many_all_inputs_no_data_returns_some_empty() {
+        let r1: ValidationResult<i32, String> = ValidationResult::new_with_error("e1".to_string());
+        let r2: ValidationResult<i32, String> = ValidationResult::new_with_error("e2".to_string());
+
+        let merged = ValidationResult::merge_many(vec![r1, r2]);
         assert_eq!(merged.data, Some(vec![]));
+        assert_eq!(merged.errors, vec!["e1".to_string(), "e2".to_string()]);
+    }
+
+    // -- flatten_strict() (issue #2867 fix) --
+    // PROTOCOL_VERSION_12+ uses these. They restore the
+    // `data.is_none() ⇔ no work done` invariant.
+
+    #[test]
+    fn test_flatten_strict_merges_non_empty_data() {
+        let r1: ValidationResult<Vec<i32>, String> = ValidationResult::new_with_data(vec![1, 2]);
+        let r2: ValidationResult<Vec<i32>, String> =
+            ValidationResult::new_with_data_and_errors(vec![3], vec!["e".to_string()]);
+        let r3: ValidationResult<Vec<i32>, String> =
+            ValidationResult::new_with_error("e2".to_string());
+
+        let flat = ValidationResult::flatten_strict(vec![r1, r2, r3]);
+        assert_eq!(flat.data, Some(vec![1, 2, 3]));
+        assert_eq!(flat.errors, vec!["e".to_string(), "e2".to_string()]);
+    }
+
+    #[test]
+    fn test_flatten_strict_empty_input_returns_none_data() {
+        let flat: ValidationResult<Vec<i32>, String> =
+            ValidationResult::flatten_strict(std::iter::empty());
+        assert_eq!(flat.data, None);
+        assert!(flat.errors.is_empty());
+    }
+
+    #[test]
+    fn test_flatten_strict_all_inputs_no_data_returns_none() {
+        // The whole point of strict: when no input contributed data,
+        // return data:None — not Some(empty_vec). Downstream code
+        // (process_validation_result_v0:241) keys on data.is_none().
+        let r1: ValidationResult<Vec<i32>, String> =
+            ValidationResult::new_with_error("e1".to_string());
+        let r2: ValidationResult<Vec<i32>, String> =
+            ValidationResult::new_with_error("e2".to_string());
+
+        let flat = ValidationResult::flatten_strict(vec![r1, r2]);
+        assert!(flat.data.is_none());
+        assert_eq!(flat.errors, vec!["e1".to_string(), "e2".to_string()]);
+    }
+
+    #[test]
+    fn test_flatten_strict_some_empty_some_non_empty_returns_some() {
+        // Mixed input: one had data:Some(empty_vec), another had
+        // Some(non_empty). The aggregate is non-empty, so data:Some(...).
+        let r1: ValidationResult<Vec<i32>, String> = ValidationResult::new_with_data(vec![]);
+        let r2: ValidationResult<Vec<i32>, String> = ValidationResult::new_with_data(vec![42]);
+
+        let flat = ValidationResult::flatten_strict(vec![r1, r2]);
+        assert_eq!(flat.data, Some(vec![42]));
+        assert!(flat.errors.is_empty());
+    }
+
+    #[test]
+    fn test_flatten_strict_all_some_empty_returns_none() {
+        // All inputs had data:Some(empty_vec). The aggregate Vec is
+        // empty → data:None per the strict contract.
+        let r1: ValidationResult<Vec<i32>, String> = ValidationResult::new_with_data(vec![]);
+        let r2: ValidationResult<Vec<i32>, String> = ValidationResult::new_with_data(vec![]);
+
+        let flat = ValidationResult::flatten_strict(vec![r1, r2]);
+        assert!(flat.data.is_none());
+        assert!(flat.errors.is_empty());
+    }
+
+    // -- merge_many_strict() (issue #2867 fix) --
+
+    #[test]
+    fn test_merge_many_strict_collects_non_empty_data() {
+        let r1: ValidationResult<i32, String> = ValidationResult::new_with_data(1);
+        let r2: ValidationResult<i32, String> = ValidationResult::new_with_data(2);
+        let r3: ValidationResult<i32, String> = ValidationResult::new_with_error("e".to_string());
+
+        let merged = ValidationResult::merge_many_strict(vec![r1, r2, r3]);
+        assert_eq!(merged.data, Some(vec![1, 2]));
+        assert_eq!(merged.errors, vec!["e".to_string()]);
+    }
+
+    #[test]
+    fn test_merge_many_strict_empty_input_returns_none_data() {
+        let merged: ValidationResult<Vec<i32>, String> = ValidationResult::merge_many_strict(
+            std::iter::empty::<ValidationResult<i32, String>>(),
+        );
+        assert!(merged.data.is_none());
         assert!(merged.errors.is_empty());
     }
 
+    #[test]
+    fn test_merge_many_strict_all_inputs_no_data_returns_none() {
+        // The bug-fixing case: all per-transition results returned
+        // errors-only with no action. Strict aggregator surfaces this
+        // as data:None so the downstream paid/unpaid switch picks unpaid.
+        let r1: ValidationResult<i32, String> = ValidationResult::new_with_error("e1".to_string());
+        let r2: ValidationResult<i32, String> = ValidationResult::new_with_error("e2".to_string());
+
+        let merged = ValidationResult::merge_many_strict(vec![r1, r2]);
+        assert!(merged.data.is_none());
+        assert_eq!(merged.errors, vec!["e1".to_string(), "e2".to_string()]);
+    }
+
+    #[test]
+    fn test_merge_many_strict_some_data_returns_some() {
+        let r1: ValidationResult<i32, String> = ValidationResult::new_with_error("e1".to_string());
+        let r2: ValidationResult<i32, String> = ValidationResult::new_with_data(7);
+
+        let merged = ValidationResult::merge_many_strict(vec![r1, r2]);
+        assert_eq!(merged.data, Some(vec![7]));
+        assert_eq!(merged.errors, vec!["e1".to_string()]);
+    }
+
     // -- merge_many_errors() --
 
     #[test]

packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/mod.rs

@@ -33,6 +33,7 @@ use crate::rpc::core::CoreRPCLike;
 use crate::execution::validation::state_transition::batch::advanced_structure::v0::DocumentsBatchStateTransitionStructureValidationV0;
 use crate::execution::validation::state_transition::batch::identity_contract_nonce::v0::DocumentsBatchStateTransitionIdentityContractNonceV0;
 use crate::execution::validation::state_transition::batch::state::v0::DocumentsBatchStateTransitionStateValidationV0;
+use crate::execution::validation::state_transition::batch::transformer::v1::BatchTransitionActionTransformerV1;
 use crate::execution::validation::state_transition::processor::advanced_structure_with_state::StateTransitionStructureKnownInStateValidationV0;
 use crate::execution::validation::state_transition::processor::basic_structure::StateTransitionBasicStructureValidationV0;
 use crate::execution::validation::state_transition::processor::identity_nonces::StateTransitionIdentityNonceValidationV0;
@@ -75,9 +76,10 @@ impl StateTransitionActionTransformer for BatchTransition {
             .transform_into_action
         {
             0 => self.transform_into_action_v0(&platform.into(), block_info, validation_mode, tx),
+            1 => self.transform_into_action_v1(&platform.into(), block_info, validation_mode, tx),
             version => Err(Error::Execution(ExecutionError::UnknownVersionMismatch {
                 method: "documents batch transition: transform_into_action".to_string(),
-                known_versions: vec![0],
+                known_versions: vec![0, 1],
                 received: version,
             })),
         }

packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/state/v0/fetch_documents.rs

@@ -69,6 +69,11 @@ pub(crate) fn fetch_documents_for_transitions(
         })
         .collect::<Result<Vec<ConsensusValidationResult<Vec<Document>>>, Error>>()?;
 
+    // The deprecated non-strict aggregator is fine here: the only caller
+    // checks `is_valid()` (errors), not `data.is_some()`, so the
+    // `Some(empty_vec)` vs `None` distinction is invisible to it. See
+    // issue #2867 for context on the strict aggregators.
+    #[allow(deprecated)]
     let validation_result = ConsensusValidationResult::flatten(validation_results_of_documents);
 
     Ok(validation_result)