diff --git a/packages/rs-dpp/src/validation/validation_result/flatten/mod.rs b/packages/rs-dpp/src/validation/validation_result/flatten/mod.rs new file mode 100644 index 00000000000..0bfd3c4c7b2 --- /dev/null +++ b/packages/rs-dpp/src/validation/validation_result/flatten/mod.rs @@ -0,0 +1,2 @@ +pub(super) mod v0; +pub(super) mod v1; diff --git a/packages/rs-dpp/src/validation/validation_result/flatten/v0/mod.rs b/packages/rs-dpp/src/validation/validation_result/flatten/v0/mod.rs new file mode 100644 index 00000000000..9ba36e14e82 --- /dev/null +++ b/packages/rs-dpp/src/validation/validation_result/flatten/v0/mod.rs @@ -0,0 +1,75 @@ +//! v0 of [`ConsensusValidationResult::flatten`]. +//! +//! Legacy semantics: always returns `data: Some(Vec<...>)`, including +//! `Some(empty_vec)` when no input contributed any data. +//! +//! Preserved for `PROTOCOL_VERSION_11` and below — the +//! `Some(empty_vec)`-on-no-data behavior is part of the existing chain +//! history, and changing it would be a consensus-breaking change for +//! already-finalized blocks. New code should let the facade dispatch to v1. +//! +//! See issue #2867 for context. +//! +//! [`ConsensusValidationResult::flatten`]: crate::validation::ConsensusValidationResult::flatten + +use crate::validation::ValidationResult; +use std::fmt::Debug; + +pub(in crate::validation::validation_result) fn flatten_v0( + items: I, +) -> ValidationResult, E> +where + TData: Clone, + E: Debug, + I: IntoIterator, 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); + } + }); + ValidationResult::new_with_data_and_errors(aggregate_data, aggregate_errors) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn merges_data_and_errors() { + let r1: ValidationResult, String> = ValidationResult::new_with_data(vec![1, 2]); + let r2: ValidationResult, String> = + ValidationResult::new_with_data_and_errors(vec![3], vec!["e".to_string()]); + let r3: ValidationResult, String> = + ValidationResult::new_with_error("e2".to_string()); + + let flat = flatten_v0(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 empty_input_returns_some_empty() { + // Legacy v11 behavior: Some(empty_vec), not None. + let flat: ValidationResult, String> = + flatten_v0(std::iter::empty::, String>>()); + assert_eq!(flat.data, Some(vec![])); + assert!(flat.errors.is_empty()); + } + + #[test] + fn all_inputs_no_data_returns_some_empty() { + let r1: ValidationResult, String> = + ValidationResult::new_with_error("e1".to_string()); + let r2: ValidationResult, String> = + ValidationResult::new_with_error("e2".to_string()); + + let flat = flatten_v0(vec![r1, r2]); + assert_eq!(flat.data, Some(vec![])); + assert_eq!(flat.errors, vec!["e1".to_string(), "e2".to_string()]); + } +} diff --git a/packages/rs-dpp/src/validation/validation_result/flatten/v1/mod.rs b/packages/rs-dpp/src/validation/validation_result/flatten/v1/mod.rs new file mode 100644 index 00000000000..13a77270709 --- /dev/null +++ b/packages/rs-dpp/src/validation/validation_result/flatten/v1/mod.rs @@ -0,0 +1,121 @@ +//! v1 of [`ConsensusValidationResult::flatten`]. +//! +//! Canonical semantics: returns `data: None` when no input contributed any +//! data (i.e. every input was either `data: None` or `data: Some(empty_vec)`), +//! and `data: Some(merged_vec)` when at least one input contributed +//! non-empty data. +//! +//! This honors the invariant `data.is_none() ⇔ no work done`, which +//! downstream code (e.g. `process_validation_result_v0:241`) relies on to +//! choose between `PaidConsensusError` and `UnpaidConsensusError`. +//! +//! # Caller-intent ambiguity +//! +//! `flatten_v1` keys on `aggregate_data.is_empty()` to decide between +//! `data: None` and `data: Some(_)`. This collapses two distinct +//! caller-side intents into the same output: +//! +//! * **Truly no work**: every input had `data: None`. +//! * **Validated but produced no output**: every input had +//! `data: Some(empty_vec)`. +//! +//! v1 cannot distinguish those two cases at the aggregate level — both +//! end up as `data: None` and are routed to `UnpaidConsensusError` +//! downstream. For the documents-batch path under PROTOCOL_VERSION_12 this +//! is safe: every per-transition handler emits at least one action on +//! success and a bump action on failure, so no caller produces +//! `Some(empty_vec)`. A future caller that needs "validated, but no +//! actions to apply" must signal that with at least one non-empty entry, +//! not with `Some(empty_vec)`. +//! +//! See issue #2867 for context. +//! +//! [`ConsensusValidationResult::flatten`]: crate::validation::ConsensusValidationResult::flatten + +use crate::validation::ValidationResult; +use std::fmt::Debug; + +pub(in crate::validation::validation_result) fn flatten_v1( + items: I, +) -> ValidationResult, E> +where + TData: Clone, + E: Debug, + I: IntoIterator, 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::new_with_errors(aggregate_errors) + } else { + ValidationResult::new_with_data_and_errors(aggregate_data, aggregate_errors) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn merges_non_empty_data() { + let r1: ValidationResult, String> = ValidationResult::new_with_data(vec![1, 2]); + let r2: ValidationResult, String> = + ValidationResult::new_with_data_and_errors(vec![3], vec!["e".to_string()]); + let r3: ValidationResult, String> = + ValidationResult::new_with_error("e2".to_string()); + + let flat = flatten_v1(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 empty_input_returns_none() { + let flat: ValidationResult, String> = + flatten_v1(std::iter::empty::, String>>()); + assert_eq!(flat.data, None); + assert!(flat.errors.is_empty()); + } + + #[test] + fn all_inputs_no_data_returns_none() { + // Downstream code (process_validation_result_v0:241) keys on + // data.is_none() to route to UnpaidConsensusError. + let r1: ValidationResult, String> = + ValidationResult::new_with_error("e1".to_string()); + let r2: ValidationResult, String> = + ValidationResult::new_with_error("e2".to_string()); + + let flat = flatten_v1(vec![r1, r2]); + assert!(flat.data.is_none()); + assert_eq!(flat.errors, vec!["e1".to_string(), "e2".to_string()]); + } + + #[test] + fn some_empty_some_non_empty_returns_some() { + let r1: ValidationResult, String> = ValidationResult::new_with_data(vec![]); + let r2: ValidationResult, String> = ValidationResult::new_with_data(vec![42]); + + let flat = flatten_v1(vec![r1, r2]); + assert_eq!(flat.data, Some(vec![42])); + assert!(flat.errors.is_empty()); + } + + #[test] + fn all_some_empty_returns_none() { + // All inputs had data:Some(empty_vec). The aggregate Vec is empty → data:None. + let r1: ValidationResult, String> = ValidationResult::new_with_data(vec![]); + let r2: ValidationResult, String> = ValidationResult::new_with_data(vec![]); + + let flat = flatten_v1(vec![r1, r2]); + assert!(flat.data.is_none()); + assert!(flat.errors.is_empty()); + } +} diff --git a/packages/rs-dpp/src/validation/validation_result/merge_many/mod.rs b/packages/rs-dpp/src/validation/validation_result/merge_many/mod.rs new file mode 100644 index 00000000000..0bfd3c4c7b2 --- /dev/null +++ b/packages/rs-dpp/src/validation/validation_result/merge_many/mod.rs @@ -0,0 +1,2 @@ +pub(super) mod v0; +pub(super) mod v1; diff --git a/packages/rs-dpp/src/validation/validation_result/merge_many/v0/mod.rs b/packages/rs-dpp/src/validation/validation_result/merge_many/v0/mod.rs new file mode 100644 index 00000000000..a1c79b68fad --- /dev/null +++ b/packages/rs-dpp/src/validation/validation_result/merge_many/v0/mod.rs @@ -0,0 +1,71 @@ +//! v0 of [`ValidationResult::merge_many`]. +//! +//! Legacy semantics: always returns `data: Some(Vec<...>)`, including +//! `Some(empty_vec)` when no input had `data: Some(_)`. +//! +//! Preserved for `PROTOCOL_VERSION_11` and below — the +//! `Some(empty_vec)`-on-no-data behavior is part of the existing chain +//! history, and changing it would be a consensus-breaking change for +//! already-finalized blocks. New code should let the facade dispatch to v1. +//! +//! See issue #2867 for context. +//! +//! [`ValidationResult::merge_many`]: crate::validation::ValidationResult::merge_many + +use crate::validation::ValidationResult; +use std::fmt::Debug; + +pub(in crate::validation::validation_result) fn merge_many_v0( + items: I, +) -> ValidationResult, E> +where + TData: Clone, + E: Debug, + I: IntoIterator>, +{ + 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); + } + }); + ValidationResult::new_with_data_and_errors(aggregate_data, aggregate_errors) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn collects_data_into_vec() { + let r1: ValidationResult = ValidationResult::new_with_data(1); + let r2: ValidationResult = ValidationResult::new_with_data(2); + let r3: ValidationResult = ValidationResult::new_with_error("e".to_string()); + + let merged = merge_many_v0(vec![r1, r2, r3]); + assert_eq!(merged.data, Some(vec![1, 2])); + assert_eq!(merged.errors, vec!["e".to_string()]); + } + + #[test] + fn empty_input_returns_some_empty() { + // Legacy v11 behavior: Some(empty_vec), not None. + let merged: ValidationResult, String> = + merge_many_v0(std::iter::empty::>()); + assert_eq!(merged.data, Some(vec![])); + assert!(merged.errors.is_empty()); + } + + #[test] + fn all_inputs_no_data_returns_some_empty() { + let r1: ValidationResult = ValidationResult::new_with_error("e1".to_string()); + let r2: ValidationResult = ValidationResult::new_with_error("e2".to_string()); + + let merged = merge_many_v0(vec![r1, r2]); + assert_eq!(merged.data, Some(vec![])); + assert_eq!(merged.errors, vec!["e1".to_string(), "e2".to_string()]); + } +} diff --git a/packages/rs-dpp/src/validation/validation_result/merge_many/v1/mod.rs b/packages/rs-dpp/src/validation/validation_result/merge_many/v1/mod.rs new file mode 100644 index 00000000000..0a4135cefae --- /dev/null +++ b/packages/rs-dpp/src/validation/validation_result/merge_many/v1/mod.rs @@ -0,0 +1,95 @@ +//! v1 of [`ValidationResult::merge_many`]. +//! +//! Canonical semantics: returns `data: None` when no input had +//! `data: Some(_)`, and `data: Some(Vec)` when at least one input +//! contributed data. +//! +//! This honors the invariant `data.is_none() ⇔ no work done`, which +//! downstream code (e.g. `process_validation_result_v0:241`) relies on to +//! choose between `PaidConsensusError` and `UnpaidConsensusError`. +//! +//! # Caller-intent ambiguity +//! +//! `merge_many_v1` keys on `aggregate_data.is_empty()` to decide between +//! `data: None` and `data: Some(_)`. Every `Some(_)` input contributes one +//! element to `aggregate_data`, so the only way to get `data: None` is to +//! have zero inputs with `data: Some(_)`. There is no `Some(empty_vec)` +//! input shape at this layer (the per-item `data` is `TData`, not +//! `Vec`), so the collapse hazard described for `flatten_v1` +//! doesn't apply here. The dispatcher facade ([`ValidationResult::merge_many`]) +//! shares the limitation note for symmetry. +//! +//! See issue #2867 for context. +//! +//! [`ValidationResult::merge_many`]: crate::validation::ValidationResult::merge_many + +use crate::validation::ValidationResult; +use std::fmt::Debug; + +pub(in crate::validation::validation_result) fn merge_many_v1( + items: I, +) -> ValidationResult, E> +where + TData: Clone, + E: Debug, + I: IntoIterator>, +{ + 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::new_with_errors(aggregate_errors) + } else { + ValidationResult::new_with_data_and_errors(aggregate_data, aggregate_errors) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn collects_non_empty_data() { + let r1: ValidationResult = ValidationResult::new_with_data(1); + let r2: ValidationResult = ValidationResult::new_with_data(2); + let r3: ValidationResult = ValidationResult::new_with_error("e".to_string()); + + let merged = merge_many_v1(vec![r1, r2, r3]); + assert_eq!(merged.data, Some(vec![1, 2])); + assert_eq!(merged.errors, vec!["e".to_string()]); + } + + #[test] + fn empty_input_returns_none() { + let merged: ValidationResult, String> = + merge_many_v1(std::iter::empty::>()); + assert!(merged.data.is_none()); + assert!(merged.errors.is_empty()); + } + + #[test] + fn all_inputs_no_data_returns_none() { + let r1: ValidationResult = ValidationResult::new_with_error("e1".to_string()); + let r2: ValidationResult = ValidationResult::new_with_error("e2".to_string()); + + let merged = merge_many_v1(vec![r1, r2]); + assert!(merged.data.is_none()); + assert_eq!(merged.errors, vec!["e1".to_string(), "e2".to_string()]); + } + + #[test] + fn some_data_returns_some() { + let r1: ValidationResult = ValidationResult::new_with_error("e1".to_string()); + let r2: ValidationResult = ValidationResult::new_with_data(7); + + let merged = merge_many_v1(vec![r1, r2]); + assert_eq!(merged.data, Some(vec![7])); + assert_eq!(merged.errors, vec!["e1".to_string()]); + } +} diff --git a/packages/rs-dpp/src/validation/validation_result.rs b/packages/rs-dpp/src/validation/validation_result/mod.rs similarity index 79% rename from packages/rs-dpp/src/validation/validation_result.rs rename to packages/rs-dpp/src/validation/validation_result/mod.rs index 505e65edef4..73714d808cf 100644 --- a/packages/rs-dpp/src/validation/validation_result.rs +++ b/packages/rs-dpp/src/validation/validation_result/mod.rs @@ -1,7 +1,11 @@ use crate::errors::consensus::ConsensusError; +use crate::version::PlatformVersion; use crate::ProtocolError; use std::fmt::Debug; +mod flatten; +mod merge_many; + #[macro_export] macro_rules! check_validation_result_with_data { ($result:expr) => { @@ -34,36 +38,79 @@ impl Default for ValidationResult { } impl ValidationResult, E> { + /// Aggregate a list of `ValidationResult, E>` into a single + /// result. Dispatches to the version selected by `platform_version`: + /// + /// - **v0** (`PROTOCOL_VERSION_11` and below): always returns + /// `data: Some(Vec<...>)`, including `Some(empty_vec)` when no input + /// contributed any data. Preserved for chain reproducibility. + /// - **v1** (`PROTOCOL_VERSION_12`+): returns `data: None` when no input + /// contributed any data. Honors the invariant + /// `data.is_none() ⇔ no work done`, which downstream code (e.g. + /// `process_validation_result_v0:241`) relies on to choose between + /// `PaidConsensusError` and `UnpaidConsensusError`. + /// + /// # v1 caller-intent ambiguity + /// + /// v1 keys on `aggregate_data.is_empty()` to decide between + /// `data: None` and `data: Some(_)`, which collapses two distinct + /// caller intents into the same output: every input had `data: None` + /// (truly no work) and every input had `data: Some(empty_vec)` + /// (validated but produced no output). v1 cannot distinguish those + /// at the aggregate level — both yield `data: None` and are routed + /// to `UnpaidConsensusError` downstream. Callers that need "validated + /// but no actions" must signal that with at least one non-empty entry. + /// + /// See issue #2867 for context on the v0 → v1 change. pub fn flatten, E>>>( items: I, - ) -> ValidationResult, 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); - } - }); - ValidationResult::new_with_data_and_errors(aggregate_data, aggregate_errors) + platform_version: &PlatformVersion, + ) -> Result, E>, ProtocolError> { + match platform_version.dpp.validation.validation_result.flatten { + 0 => Ok(flatten::v0::flatten_v0(items)), + 1 => Ok(flatten::v1::flatten_v1(items)), + version => Err(ProtocolError::UnknownVersionMismatch { + method: "ValidationResult::flatten".to_string(), + known_versions: vec![0, 1], + received: version, + }), + } } } impl ValidationResult { + /// Aggregate a list of `ValidationResult` into a + /// `ValidationResult, E>`. Dispatches to the version selected + /// by `platform_version`: + /// + /// - **v0** (`PROTOCOL_VERSION_11` and below): always returns + /// `data: Some(Vec<...>)`, including `Some(empty_vec)` when no input + /// contributed any data. Preserved for chain reproducibility. + /// - **v1** (`PROTOCOL_VERSION_12`+): returns `data: None` when no input + /// contributed any data. See [`flatten`] for the invariant this + /// restores. + /// + /// Unlike [`flatten`], `merge_many` operates on per-item `TData` (not + /// `Vec`), so each `Some(_)` input contributes exactly one + /// element — there is no `Some(empty_vec)`-input collapse hazard at + /// this layer. + /// + /// See issue #2867 for context on the v0 → v1 change. + /// + /// [`flatten`]: ValidationResult::flatten pub fn merge_many>>( items: I, - ) -> ValidationResult, 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); - } - }); - ValidationResult::new_with_data_and_errors(aggregate_data, aggregate_errors) + platform_version: &PlatformVersion, + ) -> Result, E>, ProtocolError> { + match platform_version.dpp.validation.validation_result.merge_many { + 0 => Ok(merge_many::v0::merge_many_v0(items)), + 1 => Ok(merge_many::v1::merge_many_v1(items)), + version => Err(ProtocolError::UnknownVersionMismatch { + method: "ValidationResult::merge_many".to_string(), + known_versions: vec![0, 1], + received: version, + }), + } } } @@ -539,48 +586,46 @@ mod tests { assert_eq!(result.errors, vec!["bad".to_string()]); } - // -- flatten() -- + // -- facade dispatch (flatten / merge_many take platform_version) -- + // + // These verify the version field on PlatformVersion correctly steers the + // facade to v0 vs v1 semantics. Per-version behavior is tested in each + // version's own module (e.g. `flatten::v1::tests`). #[test] - fn test_flatten_merges_data_and_errors() { - let r1: ValidationResult, String> = ValidationResult::new_with_data(vec![1, 2]); - let r2: ValidationResult, String> = - ValidationResult::new_with_data_and_errors(vec![3], vec!["e".to_string()]); - let r3: ValidationResult, String> = - ValidationResult::new_with_error("e2".to_string()); - - let flat = ValidationResult::flatten(vec![r1, r2, r3]); - assert_eq!(flat.data, Some(vec![1, 2, 3])); - assert_eq!(flat.errors, vec!["e".to_string(), "e2".to_string()]); + fn test_facade_flatten_v0_returns_some_empty_on_no_data() { + // PROTOCOL_VERSION_11 maps to dpp.validation.validation_result.flatten = 0 + let pv = PlatformVersion::get(11).expect("v11 exists"); + let r1: ValidationResult, ConsensusError> = + ValidationResult::new_with_errors(vec![]); + let flat = ValidationResult::flatten(vec![r1], pv).expect("dispatch ok"); + assert_eq!(flat.data, Some(vec![])); } #[test] - fn test_flatten_empty_input() { - let flat: ValidationResult, String> = - ValidationResult::flatten(std::iter::empty()); - assert_eq!(flat.data, Some(vec![])); - assert!(flat.errors.is_empty()); + fn test_facade_flatten_v1_returns_none_on_no_data() { + // PROTOCOL_VERSION_12 maps to dpp.validation.validation_result.flatten = 1 + let pv = PlatformVersion::get(12).expect("v12 exists"); + let r1: ValidationResult, ConsensusError> = + ValidationResult::new_with_errors(vec![]); + let flat = ValidationResult::flatten(vec![r1], pv).expect("dispatch ok"); + assert!(flat.data.is_none()); } - // -- merge_many() -- - #[test] - fn test_merge_many_collects_data_into_vec() { - let r1: ValidationResult = ValidationResult::new_with_data(1); - let r2: ValidationResult = ValidationResult::new_with_data(2); - let r3: ValidationResult = ValidationResult::new_with_error("e".to_string()); - - let merged = ValidationResult::merge_many(vec![r1, r2, r3]); - assert_eq!(merged.data, Some(vec![1, 2])); - assert_eq!(merged.errors, vec!["e".to_string()]); + fn test_facade_merge_many_v0_returns_some_empty_on_no_data() { + let pv = PlatformVersion::get(11).expect("v11 exists"); + let r1: ValidationResult = ValidationResult::new_with_errors(vec![]); + let merged = ValidationResult::merge_many(vec![r1], pv).expect("dispatch ok"); + assert_eq!(merged.data, Some(vec![])); } #[test] - fn test_merge_many_empty_input() { - let merged: ValidationResult, String> = - ValidationResult::merge_many(std::iter::empty::>()); - assert_eq!(merged.data, Some(vec![])); - assert!(merged.errors.is_empty()); + fn test_facade_merge_many_v1_returns_none_on_no_data() { + let pv = PlatformVersion::get(12).expect("v12 exists"); + let r1: ValidationResult = ValidationResult::new_with_errors(vec![]); + let merged = ValidationResult::merge_many(vec![r1], pv).expect("dispatch ok"); + assert!(merged.data.is_none()); } // -- merge_many_errors() -- diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/state/v0/fetch_documents.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/state/v0/fetch_documents.rs index 79f1b87ec29..58583fb5455 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/state/v0/fetch_documents.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/state/v0/fetch_documents.rs @@ -69,7 +69,8 @@ pub(crate) fn fetch_documents_for_transitions( }) .collect::>>, Error>>()?; - let validation_result = ConsensusValidationResult::flatten(validation_results_of_documents); + let validation_result = + ConsensusValidationResult::flatten(validation_results_of_documents, platform_version)?; Ok(validation_result) } diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/nft.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/nft.rs index ce43810cdd0..9498b00ed25 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/nft.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/nft.rs @@ -1662,10 +1662,18 @@ mod nft_tests { assert_eq!(buyers_balance, dash_to_credits!(0.9) - 68691480); } - #[tokio::test] - async fn test_document_set_price_and_try_purchase_at_different_amount() { - let platform_version = PlatformVersion::latest(); + /// Helper for the paired Purchase-at-wrong-price test. Same scenario at + /// PROTOCOL_VERSION_11 (legacy bump-only fee — Purchase paths don't emit + /// per-tx bumps in v0 of the transformer) and PROTOCOL_VERSION_12+ (bump + /// emission active for every per-tx failure). Both versions land as + /// PaidConsensusError; only the fee differs. + async fn run_document_set_price_and_try_purchase_at_different_amount_at_protocol_version( + protocol_version: dpp::version::ProtocolVersion, + ) { + let platform_version = PlatformVersion::get(protocol_version) + .expect("expected platform version for the requested protocol_version"); let (mut platform, contract) = TestPlatformBuilder::new() + .with_initial_protocol_version(protocol_version) .build_with_mock_rpc() .set_initial_state_structure() .with_crypto_card_game_nft(TradeMode::DirectPurchase); @@ -1847,7 +1855,12 @@ mod nft_tests { .unwrap() .expect("expected to commit transaction"); - assert_eq!(processing_result.invalid_paid_count(), 1); + assert_eq!( + processing_result.invalid_paid_count(), + 1, + "PROTOCOL_VERSION_{}: must land as PaidConsensusError", + protocol_version, + ); let result = processing_result.into_execution_results().remove(0); @@ -1861,6 +1874,23 @@ mod nft_tests { assert_eq!(consensus_error.to_string(), "5rJccTdtJfg6AxSKyrptWUug3PWjveEitTTLqBn9wHdk document can not be purchased for 35000000000, it's sale price is 50000000000 (in credits)"); } + /// PROTOCOL_VERSION_12+: bump emission active on Purchase failure paths. + #[tokio::test] + async fn test_document_set_price_and_try_purchase_at_different_amount() { + run_document_set_price_and_try_purchase_at_different_amount_at_protocol_version( + PlatformVersion::latest().protocol_version, + ) + .await; + } + + /// PROTOCOL_VERSION_11: legacy bump-only fee (Purchase paths don't emit + /// per-tx bumps in v0 of the transformer; the empty action still flows as + /// PaidConsensusError via the legacy `Some(empty_vec)` aggregator). + #[tokio::test] + async fn test_document_set_price_and_try_purchase_at_different_amount_protocol_version_11() { + run_document_set_price_and_try_purchase_at_different_amount_at_protocol_version(11).await; + } + #[tokio::test] async fn test_document_set_price_and_purchase_from_ones_self() { let platform_version = PlatformVersion::latest(); @@ -2057,12 +2087,19 @@ mod nft_tests { assert_eq!(consensus_error.to_string(), "Document transition action on document type: card identity trying to purchase a document that is already owned by the purchaser is not supported"); } - #[tokio::test] - async fn test_document_set_price_and_purchase_then_try_buy_back() { + /// Helper for the paired Purchase-then-buy-back test. Same scenario at + /// PROTOCOL_VERSION_11 (legacy bump-only fee) and PROTOCOL_VERSION_12+ + /// (bump emission active for every per-tx failure). Both versions land + /// as PaidConsensusError; only the fee differs. + async fn run_document_set_price_and_purchase_then_try_buy_back_at_protocol_version( + protocol_version: dpp::version::ProtocolVersion, + ) { // In this test we try to buy back a document after it has been sold - let platform_version = PlatformVersion::latest(); + let platform_version = PlatformVersion::get(protocol_version) + .expect("expected platform version for the requested protocol_version"); let (mut platform, contract) = TestPlatformBuilder::new() + .with_initial_protocol_version(protocol_version) .build_with_mock_rpc() .set_initial_state_structure() .with_crypto_card_game_nft(TradeMode::DirectPurchase); @@ -2355,7 +2392,12 @@ mod nft_tests { .unwrap() .expect("expected to commit transaction"); - assert_eq!(processing_result.invalid_paid_count(), 1); + assert_eq!( + processing_result.invalid_paid_count(), + 1, + "PROTOCOL_VERSION_{}: must land as PaidConsensusError", + protocol_version, + ); let result = processing_result.into_execution_results().remove(0); @@ -2372,6 +2414,23 @@ mod nft_tests { ); } + /// PROTOCOL_VERSION_12+: bump emission active on Purchase failure paths. + #[tokio::test] + async fn test_document_set_price_and_purchase_then_try_buy_back() { + run_document_set_price_and_purchase_then_try_buy_back_at_protocol_version( + PlatformVersion::latest().protocol_version, + ) + .await; + } + + /// PROTOCOL_VERSION_11: legacy bump-only fee (Purchase paths don't emit + /// per-tx bumps in v0 of the transformer; the empty action still flows as + /// PaidConsensusError via the legacy `Some(empty_vec)` aggregator). + #[tokio::test] + async fn test_document_set_price_and_purchase_then_try_buy_back_protocol_version_11() { + run_document_set_price_and_purchase_then_try_buy_back_at_protocol_version(11).await; + } + #[tokio::test] async fn test_document_set_price_and_purchase_with_enough_credits_to_buy_but_not_enough_to_pay_for_processing( ) { @@ -2652,10 +2711,28 @@ mod nft_tests { assert_eq!(processing_result.aggregated_fees().processing_fee, 0); } - #[tokio::test] - async fn test_document_set_price_on_not_owned_document() { - let platform_version = PlatformVersion::latest(); + /// Helper for the paired set-price-on-not-owned-document test. + /// + /// - **PROTOCOL_VERSION_11**: lands as `PaidConsensusError` via the + /// legacy `flatten_v0` / `merge_many_v0` aggregators lifting the + /// empty errors-only result to `Some(empty_vec)` — preserved for + /// chain reproducibility. The contract nonce is *not* actually + /// advanced (no bump action's drive op is created). + /// - **PROTOCOL_VERSION_12+**: lands as `PaidConsensusError` because + /// the per-tx failure path now emits a + /// `BumpIdentityDataContractNonce` action + /// (`failed_per_transition_action: 1`). The contract nonce + /// advances and the user pays for the fetch + ownership check. + /// + /// Only the fee differs between the two versions. + async fn run_document_set_price_on_not_owned_document_at_protocol_version( + protocol_version: dpp::version::ProtocolVersion, + expected_processing_fee: dpp::fee::Credits, + ) { + let platform_version = PlatformVersion::get(protocol_version) + .expect("expected platform version for the requested protocol_version"); let (mut platform, contract) = TestPlatformBuilder::new() + .with_initial_protocol_version(protocol_version) .build_with_mock_rpc() .set_initial_state_structure() .with_crypto_card_game_nft(TradeMode::DirectPurchase); @@ -2782,13 +2859,34 @@ mod nft_tests { .unwrap() .expect("expected to commit transaction"); - assert_eq!(processing_result.invalid_paid_count(), 1); - + // UpdatePrice on a not-owned doc: the per-tx ownership check fails. + // Both protocol versions land as PaidConsensusError, but for + // different reasons: + // - v11: `failed_per_transition_action` is 0, helper returns + // errors-only; legacy `flatten_v0`/`merge_many_v0` lift to + // `Some(empty_vec)`, recorded as Paid with the bump-only fee + // (no actual nonce advance — the v11 footgun preserved for + // chain reproducibility). + // - v12: helper emits `BumpIdentityDataContractNonce`; aggregator + // wraps as `Some([bump])`; recorded as Paid; nonce advances. + // See the helper doc on + // `run_document_set_price_on_not_owned_document_at_protocol_version` + // for the full split. + assert_eq!( + processing_result.invalid_paid_count(), + 1, + "PROTOCOL_VERSION_{}: ownership-mismatch UpdatePrice must land as PaidConsensusError", + protocol_version, + ); assert_eq!(processing_result.invalid_unpaid_count(), 0); - assert_eq!(processing_result.valid_count(), 0); - assert_eq!(processing_result.aggregated_fees().processing_fee, 36200); + assert_eq!( + processing_result.aggregated_fees().processing_fee, + expected_processing_fee, + "PROTOCOL_VERSION_{}: processing fee must match the version-specific baseline", + protocol_version, + ); let sender_documents_sql_string = format!("select * from card where $ownerId == '{}'", identity.id()); @@ -2818,6 +2916,24 @@ mod nft_tests { ); } + /// PROTOCOL_VERSION_12+: bump emission charges the user for the fetch + + /// ownership check that ran before the failure. + #[tokio::test] + async fn test_document_set_price_on_not_owned_document() { + run_document_set_price_on_not_owned_document_at_protocol_version( + PlatformVersion::latest().protocol_version, + 571240, + ) + .await; + } + + /// PROTOCOL_VERSION_11: pre-fix bump-only fee. Pinned so v11 chain + /// history stays bit-for-bit reproducible. + #[tokio::test] + async fn test_document_set_price_on_not_owned_document_protocol_version_11() { + run_document_set_price_on_not_owned_document_at_protocol_version(11, 36200).await; + } + #[tokio::test] async fn test_document_set_price_and_purchase_with_token_costs() { let platform_version = PlatformVersion::latest(); diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/replacement.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/replacement.rs index 759a1f0f13f..8a16213a46b 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/replacement.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/replacement.rs @@ -509,11 +509,17 @@ mod replacement_tests { .await; } - #[tokio::test] - async fn test_document_replace_on_document_type_that_is_not_mutable() { - let platform_version = PlatformVersion::latest(); + /// Helper for the paired Replace-on-immutable-doc test. The same scenario + /// is exercised at PROTOCOL_VERSION_11 (legacy bump-only fee) and at + /// PROTOCOL_VERSION_12 (fee covers fetch + validation). + async fn run_document_replace_on_document_type_that_is_not_mutable_at_protocol_version( + protocol_version: dpp::version::ProtocolVersion, + expected_processing_fee: dpp::fee::Credits, + ) { + let platform_version = PlatformVersion::get(protocol_version) + .expect("expected platform version for the requested protocol_version"); let mut platform = TestPlatformBuilder::new() - .with_latest_protocol_version() + .with_initial_protocol_version(protocol_version) .build_with_mock_rpc() .set_genesis_state(); @@ -651,7 +657,262 @@ mod replacement_tests { assert_eq!(processing_result.valid_count(), 0); - assert_eq!(processing_result.aggregated_fees().processing_fee, 41880); + assert_eq!( + processing_result.aggregated_fees().processing_fee, + expected_processing_fee, + "PROTOCOL_VERSION_{}: processing fee must match the version-specific baseline", + protocol_version, + ); + } + + /// PROTOCOL_VERSION_12+: bump emission charges the user for the fetch + + /// structure validation that ran before the failure. + #[tokio::test] + async fn test_document_replace_on_document_type_that_is_not_mutable() { + run_document_replace_on_document_type_that_is_not_mutable_at_protocol_version( + PlatformVersion::latest().protocol_version, + 445700, + ) + .await; + } + + /// PROTOCOL_VERSION_11: pre-fix bump-only fee (no charge for the fetch + /// + validation work). Pinned so v11 chain history stays bit-for-bit + /// reproducible. + #[tokio::test] + async fn test_document_replace_on_document_type_that_is_not_mutable_protocol_version_11() { + run_document_replace_on_document_type_that_is_not_mutable_at_protocol_version(11, 41880) + .await; + } + + /// Pins the bump-emission contract on Replace's revision-mismatch path. + /// + /// Without the bump, a failed Replace returns errors-only with no action. + /// Fee accounting then charges the user (PaidConsensusError) but the + /// identity_contract_nonce in state never advances — the same exact bytes + /// can be re-broadcast indefinitely. + /// + /// The test asserts: + /// 1. After a Replace that fails `check_revision_is_bumped_by_one`, the + /// stored contract nonce MUST advance past the submitted nonce. + /// 2. Re-submitting the same bytes through CheckTx FirstTimeCheck MUST + /// be rejected with `InvalidIdentityNonceError`. + #[tokio::test] + async fn replayed_failed_replace_with_consumed_nonce_must_be_rejected_at_check_tx() { + use crate::execution::check_tx::CheckTxLevel; + use crate::execution::validation::state_transition::check_tx_verification::state_transition_to_execution_event_for_check_tx; + use crate::platform_types::platform::PlatformRef; + use dpp::serialization::PlatformDeserializable; + use dpp::state_transition::StateTransition; + + let platform_version = PlatformVersion::latest(); + let mut platform = TestPlatformBuilder::new() + .with_latest_protocol_version() + .build_with_mock_rpc() + .set_genesis_state(); + + let mut rng = StdRng::seed_from_u64(437); + + let platform_state = platform.state.load(); + + let (identity, signer, key) = setup_identity(&mut platform, 958, dash_to_credits!(0.5)); + + let dashpay = platform.drive.cache.system_data_contracts.load_dashpay(); + let dashpay_contract = dashpay.clone(); + + // Use the mutable `profile` doc type — same contract-and-doc-type that + // mainnet 35C0 was operating on (DPNS-like profile-replace flow). + let profile = dashpay_contract + .document_type_for_name("profile") + .expect("expected a profile document type"); + assert!(profile.documents_mutable()); + + let entropy = Bytes32::random_with_rng(&mut rng); + let mut document = profile + .random_document_with_identifier_and_entropy( + &mut rng, + identity.id(), + entropy, + DocumentFieldFillType::FillIfNotRequired, + DocumentFieldFillSize::AnyDocumentFillSize, + platform_version, + ) + .expect("expected a random document"); + // Random fillers can produce a non-URI avatarUrl that fails JSON-schema + // validation on Create. Pin it to a valid URI like the sibling tests do. + document.set("avatarUrl", "http://test.com/bob.jpg".into()); + document.set("displayName", "Original".into()); + + // 1) Create at nonce 2 — consumes nonce 2; doc lands at revision 1. + let create_transition = BatchTransition::new_document_creation_transition_from_document( + document.clone(), + profile, + entropy.0, + &key, + 2, + 0, + None, + &signer, + platform_version, + None, + ) + .await + .expect("expected to build create transition"); + + let create_serialized = create_transition + .serialize_to_bytes() + .expect("expected to serialize create"); + + let transaction = platform.drive.grove.start_transaction(); + let create_result = platform + .platform + .process_raw_state_transitions( + &vec![create_serialized], + &platform_state, + &BlockInfo::default(), + &transaction, + platform_version, + false, + None, + ) + .expect("expected to process create"); + assert_eq!(create_result.valid_count(), 1); + platform + .drive + .grove + .commit_transaction(transaction) + .unwrap() + .expect("expected to commit create"); + + let (post_create_nonce_raw, _) = platform + .drive + .fetch_identity_contract_nonce_with_fees( + identity.id().to_buffer(), + dashpay_contract.id().to_buffer(), + &BlockInfo::default(), + true, + None, + platform_version, + ) + .expect("expected to fetch contract nonce after create"); + let post_create_nonce = + post_create_nonce_raw.expect("contract nonce must be present after create"); + + // 2) Build a Replace at nonce 3 with revision 3. Doc is at revision + // 1, so check_revision_is_bumped_by_one_during_replace_v0 returns + // InvalidDocumentRevisionError(Some(1), 3) and we hit the + // failure-with-bump path in the transformer. + let mut altered_document = document.clone(); + altered_document.set_revision(Some(3)); + altered_document.set("displayName", "Out of order".into()); + + let replace_transition = + BatchTransition::new_document_replacement_transition_from_document( + altered_document, + profile, + &key, + 3, + 0, + None, + &signer, + platform_version, + None, + ) + .await + .expect("expected to build replace transition"); + + let replace_serialized = replace_transition + .serialize_to_bytes() + .expect("expected to serialize replace"); + + let transaction = platform.drive.grove.start_transaction(); + let replace_result = platform + .platform + .process_raw_state_transitions( + &vec![replace_serialized.clone()], + &platform_state, + &BlockInfo::default(), + &transaction, + platform_version, + false, + None, + ) + .expect("expected to process replace"); + platform + .drive + .grove + .commit_transaction(transaction) + .unwrap() + .expect("expected to commit failed replace"); + + assert_eq!( + replace_result.invalid_paid_count(), + 1, + "Replace must commit as invalid_paid (PaidConsensusError); execution_results={:?}", + replace_result.execution_results() + ); + assert_eq!(replace_result.valid_count(), 0); + + // 3) Direct invariant: the bump must have advanced the contract nonce + // in state. If the stored nonce is still post-create, the bump + // silently dropped — that is the bug. + let (post_replace_nonce_raw, _) = platform + .drive + .fetch_identity_contract_nonce_with_fees( + identity.id().to_buffer(), + dashpay_contract.id().to_buffer(), + &BlockInfo::default(), + true, + None, + platform_version, + ) + .expect("expected to fetch contract nonce after failed replace"); + let post_replace_nonce = + post_replace_nonce_raw.expect("contract nonce must be present after failed replace"); + + assert_ne!( + post_replace_nonce, post_create_nonce, + "failed Replace's bump action did not advance the contract \ + nonce — stored nonce is still {:#x} (= post-create value), so \ + the same serialized bytes can be replayed", + post_create_nonce + ); + + // 4) Re-submitting identical bytes through CheckTx FirstTimeCheck must + // hit the nonce check first and reject. + let replayed_state_transition = + StateTransition::deserialize_from_bytes(&replace_serialized) + .expect("expected to deserialize replayed transition"); + + let platform_state = platform.state.load(); + let platform_ref = PlatformRef { + drive: &platform.drive, + state: &platform_state, + config: &platform.config, + core_rpc: &platform.core_rpc, + }; + + let check_tx_result = state_transition_to_execution_event_for_check_tx( + &platform_ref, + replayed_state_transition, + CheckTxLevel::FirstTimeCheck, + platform_version, + ) + .expect("expected check_tx to not return an Err"); + + assert!( + !check_tx_result.is_valid(), + "CheckTx FirstTimeCheck must reject identical bytes after the \ + failed-Replace bump consumed the nonce" + ); + assert!( + check_tx_result.errors.iter().any(|e| matches!( + e, + ConsensusError::StateError(StateError::InvalidIdentityNonceError(_)) + )), + "expected InvalidIdentityNonceError on replay; got {:?}", + check_tx_result.errors + ); } #[tokio::test] @@ -851,11 +1112,21 @@ mod replacement_tests { assert_eq!(query_receiver_results.documents().len(), 0); } - #[tokio::test] - async fn test_document_replace_that_does_not_yet_exist() { - let platform_version = PlatformVersion::latest(); + /// Helper for the paired Replace-on-missing-document test. + /// + /// Both versions land as PaidConsensusError because the Replace + /// missing-target-document path emits a `BumpIdentityDataContractNonce` + /// action on every protocol version (it was the one legacy v0 bump + /// site, preserved to keep PROTOCOL_VERSION_11 chain replay bit-for-bit + /// reproducible). Only the fee differs. + async fn run_document_replace_that_does_not_yet_exist_at_protocol_version( + protocol_version: dpp::version::ProtocolVersion, + expected_processing_fee: dpp::fee::Credits, + ) { + let platform_version = PlatformVersion::get(protocol_version) + .expect("expected platform version for the requested protocol_version"); let mut platform = TestPlatformBuilder::new() - .with_latest_protocol_version() + .with_initial_protocol_version(protocol_version) .build_with_mock_rpc() .set_genesis_state(); @@ -934,13 +1205,42 @@ mod replacement_tests { .unwrap() .expect("expected to commit transaction"); - assert_eq!(processing_result.invalid_paid_count(), 1); + assert_eq!( + processing_result.invalid_paid_count(), + 1, + "PROTOCOL_VERSION_{}: must land as PaidConsensusError", + protocol_version, + ); assert_eq!(processing_result.invalid_unpaid_count(), 0); assert_eq!(processing_result.valid_count(), 0); - assert_eq!(processing_result.aggregated_fees().processing_fee, 516040); + assert_eq!( + processing_result.aggregated_fees().processing_fee, + expected_processing_fee, + "PROTOCOL_VERSION_{}: processing fee must match the version-specific baseline", + protocol_version, + ); + } + + /// PROTOCOL_VERSION_12+ — same fee as v11 because the bump emission for + /// this specific path is unconditional (pre-existing legacy behavior). + #[tokio::test] + async fn test_document_replace_that_does_not_yet_exist() { + run_document_replace_that_does_not_yet_exist_at_protocol_version( + PlatformVersion::latest().protocol_version, + 516040, + ) + .await; + } + + /// PROTOCOL_VERSION_11 — pins the legacy fee + bump-emission behavior. + /// This is the one Replace failure path that already emitted a bump on + /// v11; the bump-emission helper must not strip it on v0. + #[tokio::test] + async fn test_document_replace_that_does_not_yet_exist_protocol_version_11() { + run_document_replace_that_does_not_yet_exist_at_protocol_version(11, 516040).await; } #[tokio::test] diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/transfer.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/transfer.rs index da2030502df..2df43222264 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/transfer.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/transfer.rs @@ -1123,10 +1123,17 @@ mod transfer_tests { assert_eq!(query_receiver_results.documents().len(), 0); } - #[tokio::test] - async fn test_document_transfer_that_does_not_yet_exist() { - let platform_version = PlatformVersion::latest(); + /// Helper for the paired transfer-of-missing-document test. Same scenario + /// at PROTOCOL_VERSION_11 (legacy bump-only fee) and PROTOCOL_VERSION_12 + /// (fee covers fetch + validation work). + async fn run_document_transfer_that_does_not_yet_exist_at_protocol_version( + protocol_version: dpp::version::ProtocolVersion, + expected_processing_fee: dpp::fee::Credits, + ) { + let platform_version = PlatformVersion::get(protocol_version) + .expect("expected platform version for the requested protocol_version"); let (mut platform, contract) = TestPlatformBuilder::new() + .with_initial_protocol_version(protocol_version) .build_with_mock_rpc() .set_initial_state_structure() .with_crypto_card_game_transfer_only(Transferable::Never); @@ -1256,7 +1263,12 @@ mod transfer_tests { assert_eq!(processing_result.valid_count(), 0); - assert_eq!(processing_result.aggregated_fees().processing_fee, 36200); + assert_eq!( + processing_result.aggregated_fees().processing_fee, + expected_processing_fee, + "PROTOCOL_VERSION_{}: processing fee must match the version-specific baseline", + protocol_version, + ); let query_sender_results = platform .drive @@ -1274,6 +1286,24 @@ mod transfer_tests { assert_eq!(query_receiver_results.documents().len(), 0); } + /// PROTOCOL_VERSION_12+: bump emission charges the user for the fetch + /// that ran before the failure. + #[tokio::test] + async fn test_document_transfer_that_does_not_yet_exist() { + run_document_transfer_that_does_not_yet_exist_at_protocol_version( + PlatformVersion::latest().protocol_version, + 517400, + ) + .await; + } + + /// PROTOCOL_VERSION_11: pre-fix bump-only fee. Pinned so v11 chain + /// history stays bit-for-bit reproducible. + #[tokio::test] + async fn test_document_transfer_that_does_not_yet_exist_protocol_version_11() { + run_document_transfer_that_does_not_yet_exist_at_protocol_version(11, 36200).await; + } + #[tokio::test] async fn test_document_delete_after_transfer() { let platform_version = PlatformVersion::latest(); diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/burn/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/burn/mod.rs index b6ddfa1b53e..b55b7e13523 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/burn/mod.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/burn/mod.rs @@ -180,11 +180,31 @@ mod token_burn_tests { assert_eq!(token_balance, Some(0)); } - #[tokio::test] - async fn test_token_burn_trying_to_burn_more_than_we_have() { - let platform_version = PlatformVersion::latest(); + /// Pins the *tokens-always-pay* invariant for the + /// [`ConsensusValidationResult::merge_many`] aggregator change + /// (issue #2867): an all-failed single-token-transition batch must + /// continue to land as `PaidConsensusError` on every protocol + /// version, because the token sub-transformer + /// (`try_from_borrowed_token_burn_transition_with_contract_lookup`) + /// emits a `BumpIdentityDataContractNonce` action on + /// base-validation failure, so each per-token result has + /// `data: Some([bump])` and the v1 aggregator never collapses to + /// `data: None`. + /// + /// If a future change drops the bump from the token sub-transformer, + /// the v1 aggregator would route the failure to + /// `UnpaidConsensusError` and the tx would be removed from the block + /// by `prepare_proposal` — different state-root, different + /// replay-protection behavior than every prior chain. The paired + /// `_protocol_version_11` sibling pins the same invariant under v11 + /// (legacy aggregator, but the bump emission is identical). + async fn run_token_burn_trying_to_burn_more_than_we_have_at_protocol_version( + protocol_version: dpp::version::ProtocolVersion, + ) { + let platform_version = PlatformVersion::get(protocol_version) + .expect("expected platform version for the requested protocol_version"); let mut platform = TestPlatformBuilder::new() - .with_latest_protocol_version() + .with_initial_protocol_version(protocol_version) .build_with_mock_rpc() .set_genesis_state(); @@ -271,6 +291,24 @@ mod token_burn_tests { assert_eq!(token_balance, Some(100000)); // nothing was burned } + /// PROTOCOL_VERSION_12+: pins the tokens-always-pay invariant under the + /// new v1 aggregator. + #[tokio::test] + async fn test_token_burn_trying_to_burn_more_than_we_have() { + run_token_burn_trying_to_burn_more_than_we_have_at_protocol_version( + PlatformVersion::latest().protocol_version, + ) + .await; + } + + /// PROTOCOL_VERSION_11: pins the same invariant under the legacy v0 + /// aggregator (the bump emission is identical across versions for + /// tokens, so both run paths must produce PaidConsensusError). + #[tokio::test] + async fn test_token_burn_trying_to_burn_more_than_we_have_protocol_version_11() { + run_token_burn_trying_to_burn_more_than_we_have_at_protocol_version(11).await; + } + #[tokio::test] async fn test_token_burn_gives_error_if_trying_to_burn_from_not_allowed_identity() { let platform_version = PlatformVersion::latest(); diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/transformer/v0/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/transformer/v0/mod.rs index ef7d2427950..b22235fdb29 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/transformer/v0/mod.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/transformer/v0/mod.rs @@ -1,3 +1,26 @@ +// The aggregator helpers `ConsensusValidationResult::flatten` / +// `merge_many` are versioned via `dpp.validation.validation_result` on +// `PlatformVersion`. v0 (PROTOCOL_VERSION_11 and below) preserves the +// legacy `Some(empty_vec)`-on-no-data behavior for chain reproducibility; +// v1 (PROTOCOL_VERSION_12+) returns `data: None` in that case so an +// all-failed batch flows down the unpaid path. See issue #2867. +// +// Note on the `_v0` suffix retained on the transformer functions in this +// file (`try_into_action_v0`, `transform_document_transition_v0`, etc.): +// these are version-1 in their behavior but keep the `_v0` suffix on +// purpose. Bumping the suffix would force duplicating the entire ~1100 +// line transformer body into a `v1/mod.rs` archive (the v0 file would +// have to stay verbatim to keep v11 chain replay reproducible) — the +// kind of copy-paste this PR was specifically refactored to avoid. +// Instead, protocol-version-dependent behavior is gated at a finer +// granularity by the version fields it consumes: +// * `dpp.validation.validation_result.flatten` +// * `dpp.validation.validation_result.merge_many` +// * `drive_abci...batch_state_transition.failed_per_transition_action` +// A future protocol bump that needs different aggregator or +// failure-action semantics should add another value to one of those +// fields rather than rename this file. + use std::collections::btree_map::Entry; use std::collections::BTreeMap; use std::sync::Arc; @@ -166,6 +189,12 @@ trait BatchTransitionInternalTransformerV0 { document_id: Identifier, original_document: &Document, ) -> SimpleConsensusValidationResult; + fn failed_per_transition_action( + base_transition: &dpp::state_transition::batch_transition::document_base_transition::DocumentBaseTransition, + owner_id: Identifier, + errors: Vec, + platform_version: &PlatformVersion, + ) -> Result, Error>; } impl BatchTransitionTransformerV0 for BatchTransition { @@ -273,7 +302,8 @@ impl BatchTransitionTransformerV0 for BatchTransition { validation_results.append(&mut validation_result_tokens); - let validation_result = ConsensusValidationResult::flatten(validation_results); + let validation_result = + ConsensusValidationResult::flatten(validation_results, platform_version)?; if validation_result.has_data() { let (transitions, errors) = validation_result.into_data_and_errors()?; @@ -296,6 +326,27 @@ impl BatchTransitionTransformerV0 for BatchTransition { } impl BatchTransitionInternalTransformerV0 for BatchTransition { + /// Roll up the per-token-transition results via the versioned + /// [`ConsensusValidationResult::merge_many`] facade. + /// + /// The aggregator's v0→v1 change at PROTOCOL_VERSION_12 (see issue + /// #2867) also affects this token path. **Tokens-always-pay + /// invariant**: every per-token sub-transformer + /// (`try_from_borrowed_token_*_transition_with_contract_lookup`) + /// emits a `BumpIdentityDataContractNonce` action on its + /// base-validation failure path, so each per-token result carries + /// `data: Some(...)` even when validation fails. The v1 aggregator + /// therefore never collapses an all-failed token batch to + /// `data: None`, and token failures continue to land as + /// `PaidConsensusError` (tx stays in the block, user pays for the + /// validation work) under both v11 and v12. + /// + /// A future change to a token sub-transformer that drops the bump + /// emission on a failure path would silently route that failure to + /// `UnpaidConsensusError` (tx removed from the block by + /// `prepare_proposal`). See + /// `tests/token/burn/mod.rs::test_token_burn_trying_to_burn_more_than_we_have` + /// and its `_protocol_version_11` sibling for the regression pin. fn transform_token_transitions_within_contract_v0( platform: &PlatformStateRef, data_contract_id: &Identifier, @@ -345,7 +396,8 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { ) }) .collect::>, Error>>()?; - let validation_result = ConsensusValidationResult::merge_many(validation_result); + let validation_result = + ConsensusValidationResult::merge_many(validation_result, platform_version)?; Ok(validation_result) } fn transform_document_transitions_within_contract_v0( @@ -399,7 +451,10 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { }) .collect::>>, Error>>( )?; - Ok(ConsensusValidationResult::flatten(validation_result)) + Ok(ConsensusValidationResult::flatten( + validation_result, + platform_version, + )?) } fn transform_document_transitions_within_document_type_v0( @@ -495,7 +550,8 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { let result = ConsensusValidationResult::merge_many( document_transition_actions_validation_result, - ); + platform_version, + )?; if !result.is_valid() { return Ok(result); @@ -637,7 +693,36 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { } } - /// The data contract can be of multiple difference versions + /// Per-transition handler for document arms. + /// + /// Each per-transition failure path (ownership mismatch, revision + /// mismatch, missing target document, etc.) routes through + /// [`Self::failed_per_transition_action`], whose behavior is gated + /// on the `failed_per_transition_action` version field: + /// + /// - **v0** (`PROTOCOL_VERSION_11` and below): returns errors-only + /// with no action data. The legacy `flatten_v0` / `merge_many_v0` + /// aggregators lift this to `Some(empty_vec)`, which downstream + /// code records as `PaidConsensusError` with a bump-only fee but + /// no actual `UpdateIdentityContractNonce` drive op — the + /// "free advanced-structure validation" v11 footgun. Preserved + /// here for chain reproducibility. + /// - **v1** (`PROTOCOL_VERSION_12`+): emits a + /// `BumpIdentityDataContractNonce` action so the user pays for + /// the validation work that already ran (fetch + ownership / + /// revision check) and the contract nonce advances. + /// + /// The one exception is Replace's missing-target-document path, + /// which always emits a bump inline (regardless of version) — that + /// was the one legacy v0 bump site pre-PR, kept as-is to preserve + /// v11 chain replay bit-for-bit. + /// + /// The `user_fee_increase` argument passed into each + /// `BumpIdentityDataContractNonceAction::from_borrowed_document_base_transition` + /// call is `0` deliberately: the value gets overridden by the outer + /// Documents Batch's `user_fee_increase` when the per-transition + /// action rolls up into the `BatchTransitionAction`, so any + /// per-site value would be discarded. fn transform_document_transition_v0<'a>( drive: &Drive, transaction: TransactionArg, @@ -664,24 +749,21 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { Ok(document_create_action) } DocumentTransition::Replace(document_replace_transition) => { - let mut result = ConsensusValidationResult::::new(); - let validation_result = Self::find_replaced_document_v0(transition, replaced_documents); if !validation_result.is_valid_with_data() { - // We can set the user fee increase to 0 here because it is decided by the Documents Batch instead + // Keep this bump emission inline (not via Self::failed_per_transition_action): + // it's the one legacy v0 bump site, and routing it through the helper would + // drop the bump on PROTOCOL_VERSION_11 and diverge v11 chain replay (#2867). let bump_action = BumpIdentityDataContractNonceAction::from_borrowed_document_base_transition( document_replace_transition.base(), owner_id, 0, ); - let batched_action = - BatchedTransitionAction::BumpIdentityDataContractNonce(bump_action); - return Ok(ConsensusValidationResult::new_with_data_and_errors( - batched_action, + BatchedTransitionAction::BumpIdentityDataContractNonce(bump_action), validation_result.errors, )); } @@ -695,8 +777,12 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { ); if !validation_result.is_valid() { - result.merge(validation_result); - return Ok(result); + return Self::failed_per_transition_action( + document_replace_transition.base(), + owner_id, + validation_result.errors, + platform_version, + ); } if validate_against_state { @@ -710,8 +796,12 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { ); if !validation_result.is_valid() { - result.merge(validation_result); - return Ok(result); + return Self::failed_per_transition_action( + document_replace_transition.base(), + owner_id, + validation_result.errors, + platform_version, + ); } } @@ -728,11 +818,7 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { execution_context .add_operation(ValidationOperation::PrecalculatedOperation(fee_result)); - if result.is_valid() { - Ok(document_replace_action) - } else { - Ok(result) - } + Ok(document_replace_action) } DocumentTransition::Delete(document_delete_transition) => { let (batched_action, fee_result) = DocumentDeleteTransitionAction::try_from_document_borrowed_delete_transition_with_contract_lookup(document_delete_transition, owner_id, user_fee_increase, |_identifier| { @@ -745,14 +831,16 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { Ok(batched_action) } DocumentTransition::Transfer(document_transfer_transition) => { - let mut result = ConsensusValidationResult::::new(); - let validation_result = Self::find_replaced_document_v0(transition, replaced_documents); if !validation_result.is_valid_with_data() { - result.merge(validation_result); - return Ok(result); + return Self::failed_per_transition_action( + document_transfer_transition.base(), + owner_id, + validation_result.errors, + platform_version, + ); } let original_document = validation_result.into_data()?; @@ -764,8 +852,12 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { ); if !validation_result.is_valid() { - result.merge(validation_result); - return Ok(result); + return Self::failed_per_transition_action( + document_transfer_transition.base(), + owner_id, + validation_result.errors, + platform_version, + ); } if validate_against_state { @@ -779,8 +871,12 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { ); if !validation_result.is_valid() { - result.merge(validation_result); - return Ok(result); + return Self::failed_per_transition_action( + document_transfer_transition.base(), + owner_id, + validation_result.errors, + platform_version, + ); } } @@ -797,21 +893,19 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { execution_context .add_operation(ValidationOperation::PrecalculatedOperation(fee_result)); - if result.is_valid() { - Ok(document_transfer_action) - } else { - Ok(result) - } + Ok(document_transfer_action) } DocumentTransition::UpdatePrice(document_update_price_transition) => { - let mut result = ConsensusValidationResult::::new(); - let validation_result = Self::find_replaced_document_v0(transition, replaced_documents); if !validation_result.is_valid_with_data() { - result.merge(validation_result); - return Ok(result); + return Self::failed_per_transition_action( + document_update_price_transition.base(), + owner_id, + validation_result.errors, + platform_version, + ); } let original_document = validation_result.into_data()?; @@ -823,8 +917,12 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { ); if !validation_result.is_valid() { - result.merge(validation_result); - return Ok(result); + return Self::failed_per_transition_action( + document_update_price_transition.base(), + owner_id, + validation_result.errors, + platform_version, + ); } if validate_against_state { @@ -838,8 +936,12 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { ); if !validation_result.is_valid() { - result.merge(validation_result); - return Ok(result); + return Self::failed_per_transition_action( + document_update_price_transition.base(), + owner_id, + validation_result.errors, + platform_version, + ); } } @@ -856,21 +958,19 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { execution_context .add_operation(ValidationOperation::PrecalculatedOperation(fee_result)); - if result.is_valid() { - Ok(document_update_price_action) - } else { - Ok(result) - } + Ok(document_update_price_action) } DocumentTransition::Purchase(document_purchase_transition) => { - let mut result = ConsensusValidationResult::::new(); - let validation_result = Self::find_replaced_document_v0(transition, replaced_documents); if !validation_result.is_valid_with_data() { - result.merge(validation_result); - return Ok(result); + return Self::failed_per_transition_action( + document_purchase_transition.base(), + owner_id, + validation_result.errors, + platform_version, + ); } let original_document = validation_result.into_data()?; @@ -879,21 +979,33 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { .properties() .get_optional_integer::(PRICE)? else { - result.add_error(StateError::DocumentNotForSaleError( - DocumentNotForSaleError::new(original_document.id()), - )); - return Ok(result); + return Self::failed_per_transition_action( + document_purchase_transition.base(), + owner_id, + vec![ + StateError::DocumentNotForSaleError(DocumentNotForSaleError::new( + original_document.id(), + )) + .into(), + ], + platform_version, + ); }; if listed_price != document_purchase_transition.price() { - result.add_error(StateError::DocumentIncorrectPurchasePriceError( - DocumentIncorrectPurchasePriceError::new( - original_document.id(), - document_purchase_transition.price(), - listed_price, - ), - )); - return Ok(result); + return Self::failed_per_transition_action( + document_purchase_transition.base(), + owner_id, + vec![StateError::DocumentIncorrectPurchasePriceError( + DocumentIncorrectPurchasePriceError::new( + original_document.id(), + document_purchase_transition.price(), + listed_price, + ), + ) + .into()], + platform_version, + ); } if validate_against_state { @@ -907,8 +1019,12 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { ); if !validation_result.is_valid() { - result.merge(validation_result); - return Ok(result); + return Self::failed_per_transition_action( + document_purchase_transition.base(), + owner_id, + validation_result.errors, + platform_version, + ); } } @@ -926,11 +1042,7 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { execution_context .add_operation(ValidationOperation::PrecalculatedOperation(fee_result)); - if result.is_valid() { - Ok(document_purchase_action) - } else { - Ok(result) - } + Ok(document_purchase_action) } } } @@ -1003,4 +1115,48 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { } result } + + fn failed_per_transition_action( + base_transition: &dpp::state_transition::batch_transition::document_base_transition::DocumentBaseTransition, + owner_id: Identifier, + errors: Vec, + platform_version: &PlatformVersion, + ) -> Result, Error> { + match platform_version + .drive_abci + .validation_and_processing + .state_transitions + .batch_state_transition + .failed_per_transition_action + { + // PROTOCOL_VERSION_11 and below: errors-only, no action data. + 0 => Ok(ConsensusValidationResult::new_with_errors(errors)), + // PROTOCOL_VERSION_12+: emit a `BumpIdentityDataContractNonce` action + // so the user pays for the validation work that already ran. + 1 => { + // The `0` user_fee_increase here is a placeholder. It will be + // overridden (reapplied) with the outer Documents Batch's + // `user_fee_increase` when this per-transition action rolls up + // into the `BatchTransitionAction`, so the per-site value is + // discarded — passing `0` is harmless. + let bump_action = + BumpIdentityDataContractNonceAction::from_borrowed_document_base_transition( + base_transition, + owner_id, + 0, + ); + Ok(ConsensusValidationResult::new_with_data_and_errors( + BatchedTransitionAction::BumpIdentityDataContractNonce(bump_action), + errors, + )) + } + version => Err(Error::Execution( + crate::error::execution::ExecutionError::UnknownVersionMismatch { + method: "documents batch transition: failed_per_transition_action".to_string(), + known_versions: vec![0, 1], + received: version, + }, + )), + } + } } diff --git a/packages/rs-platform-version/src/version/dpp_versions/dpp_validation_versions/mod.rs b/packages/rs-platform-version/src/version/dpp_versions/dpp_validation_versions/mod.rs index d77c94722e3..a8572c27be2 100644 --- a/packages/rs-platform-version/src/version/dpp_versions/dpp_validation_versions/mod.rs +++ b/packages/rs-platform-version/src/version/dpp_versions/dpp_validation_versions/mod.rs @@ -10,6 +10,22 @@ pub struct DPPValidationVersions { pub data_contract: DataContractValidationVersions, pub document_type: DocumentTypeValidationVersions, pub voting: VotingValidationVersions, + pub validation_result: ValidationResultMethodVersions, +} + +/// Versions of the aggregator methods on +/// [`crate::validation::ValidationResult`] (`flatten`, `merge_many`). +/// +/// Issue #2867: in v0 the aggregators returned `Some(empty_vec)` when no +/// per-item input contributed any data, which caused +/// `validating-state-transition-for-free` — empty-action batches were treated +/// as paid (and stayed in the block) instead of unpaid (removed in +/// prepare_proposal). v1 returns `None` in that case so the result correctly +/// flows down the unpaid path. +#[derive(Clone, Debug, Default)] +pub struct ValidationResultMethodVersions { + pub flatten: FeatureVersion, + pub merge_many: FeatureVersion, } #[derive(Clone, Debug, Default)] diff --git a/packages/rs-platform-version/src/version/dpp_versions/dpp_validation_versions/v1.rs b/packages/rs-platform-version/src/version/dpp_versions/dpp_validation_versions/v1.rs index 61e85ffea12..093687b24ad 100644 --- a/packages/rs-platform-version/src/version/dpp_versions/dpp_validation_versions/v1.rs +++ b/packages/rs-platform-version/src/version/dpp_versions/dpp_validation_versions/v1.rs @@ -1,6 +1,6 @@ use crate::version::dpp_versions::dpp_validation_versions::{ DPPValidationVersions, DataContractValidationVersions, DocumentTypeValidationVersions, - JsonSchemaValidatorVersions, VotingValidationVersions, + JsonSchemaValidatorVersions, ValidationResultMethodVersions, VotingValidationVersions, }; pub const DPP_VALIDATION_VERSIONS_V1: DPPValidationVersions = DPPValidationVersions { @@ -31,4 +31,8 @@ pub const DPP_VALIDATION_VERSIONS_V1: DPPValidationVersions = DPPValidationVersi allow_other_contenders_time_testing_ms: 604_800_000, // 1 week in ms for v1 (changes in v2) votes_allowed_per_masternode: 5, }, + validation_result: ValidationResultMethodVersions { + flatten: 0, + merge_many: 0, + }, }; diff --git a/packages/rs-platform-version/src/version/dpp_versions/dpp_validation_versions/v2.rs b/packages/rs-platform-version/src/version/dpp_versions/dpp_validation_versions/v2.rs index ad370c2a999..1e795f018a7 100644 --- a/packages/rs-platform-version/src/version/dpp_versions/dpp_validation_versions/v2.rs +++ b/packages/rs-platform-version/src/version/dpp_versions/dpp_validation_versions/v2.rs @@ -1,6 +1,6 @@ use crate::version::dpp_versions::dpp_validation_versions::{ DPPValidationVersions, DataContractValidationVersions, DocumentTypeValidationVersions, - JsonSchemaValidatorVersions, VotingValidationVersions, + JsonSchemaValidatorVersions, ValidationResultMethodVersions, VotingValidationVersions, }; pub const DPP_VALIDATION_VERSIONS_V2: DPPValidationVersions = DPPValidationVersions { @@ -31,4 +31,8 @@ pub const DPP_VALIDATION_VERSIONS_V2: DPPValidationVersions = DPPValidationVersi allow_other_contenders_time_testing_ms: 2_700_000, //45 minutes votes_allowed_per_masternode: 5, }, + validation_result: ValidationResultMethodVersions { + flatten: 0, + merge_many: 0, + }, }; diff --git a/packages/rs-platform-version/src/version/dpp_versions/dpp_validation_versions/v3.rs b/packages/rs-platform-version/src/version/dpp_versions/dpp_validation_versions/v3.rs index e5621fd5851..d2d4795d6fe 100644 --- a/packages/rs-platform-version/src/version/dpp_versions/dpp_validation_versions/v3.rs +++ b/packages/rs-platform-version/src/version/dpp_versions/dpp_validation_versions/v3.rs @@ -1,6 +1,6 @@ use crate::version::dpp_versions::dpp_validation_versions::{ DPPValidationVersions, DataContractValidationVersions, DocumentTypeValidationVersions, - JsonSchemaValidatorVersions, VotingValidationVersions, + JsonSchemaValidatorVersions, ValidationResultMethodVersions, VotingValidationVersions, }; pub const DPP_VALIDATION_VERSIONS_V3: DPPValidationVersions = DPPValidationVersions { @@ -32,4 +32,14 @@ pub const DPP_VALIDATION_VERSIONS_V3: DPPValidationVersions = DPPValidationVersi allow_other_contenders_time_testing_ms: 2_700_000, //45 minutes votes_allowed_per_masternode: 5, }, + // Issue #2867: bump aggregator methods to v1 — `flatten` / `merge_many` + // now return `data: None` when no input contributed any data, instead of + // the legacy `Some(empty_vec)`. Closes the + // "validating-state-transition-for-free" gap where an all-failed + // documents batch was being recorded as PaidConsensusError with an empty + // action and the same exact bytes could be replayed across blocks. + validation_result: ValidationResultMethodVersions { + flatten: 1, + merge_many: 1, + }, }; diff --git a/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/mod.rs b/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/mod.rs index 58e63961909..8b3b18edcae 100644 --- a/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/mod.rs +++ b/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/mod.rs @@ -127,6 +127,21 @@ pub struct DriveAbciDocumentsStateTransitionValidationVersions { pub revision: FeatureVersion, pub state: FeatureVersion, pub transform_into_action: FeatureVersion, + /// Versions the action emitted when a per-transition validation fails + /// inside [`transform_document_transition`]. + /// + /// - `0` (PROTOCOL_VERSION_11 and below): errors-only, no action data. + /// The empty action flowed through the legacy + /// `flatten` / `merge_many` aggregators as `Some(empty_vec)` and was + /// accounted as `PaidConsensusError`, but no `BumpIdentityDataContractNonce` + /// drive op was created — so the user only paid the bare-bump fee + /// and the contract nonce never advanced. + /// - `1` (PROTOCOL_VERSION_12+): emit a `BumpIdentityDataContractNonce` + /// action so the user pays for the validation work that already ran + /// (fetch + ownership/revision check) and the contract nonce advances. + /// + /// [`transform_document_transition`]: crate + pub failed_per_transition_action: FeatureVersion, pub data_triggers: DriveAbciValidationDataTriggerAndBindingVersions, pub is_allowed: FeatureVersion, pub document_create_transition_structure_validation: FeatureVersion, diff --git a/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v1.rs b/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v1.rs index af26fae4cf0..fce75c16330 100644 --- a/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v1.rs +++ b/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v1.rs @@ -106,6 +106,7 @@ pub const DRIVE_ABCI_VALIDATION_VERSIONS_V1: DriveAbciValidationVersions = state: 0, revision: 0, transform_into_action: 0, + failed_per_transition_action: 0, data_triggers: DriveAbciValidationDataTriggerAndBindingVersions { bindings: 0, triggers: DriveAbciValidationDataTriggerVersions { diff --git a/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v2.rs b/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v2.rs index 0991f5d79ab..ab2d160f2a3 100644 --- a/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v2.rs +++ b/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v2.rs @@ -106,6 +106,7 @@ pub const DRIVE_ABCI_VALIDATION_VERSIONS_V2: DriveAbciValidationVersions = state: 0, revision: 0, transform_into_action: 0, + failed_per_transition_action: 0, data_triggers: DriveAbciValidationDataTriggerAndBindingVersions { bindings: 0, triggers: DriveAbciValidationDataTriggerVersions { diff --git a/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v3.rs b/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v3.rs index 9d62d308b13..c80ed9f6e0d 100644 --- a/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v3.rs +++ b/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v3.rs @@ -106,6 +106,7 @@ pub const DRIVE_ABCI_VALIDATION_VERSIONS_V3: DriveAbciValidationVersions = state: 0, revision: 0, transform_into_action: 0, + failed_per_transition_action: 0, data_triggers: DriveAbciValidationDataTriggerAndBindingVersions { bindings: 0, triggers: DriveAbciValidationDataTriggerVersions { diff --git a/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v4.rs b/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v4.rs index 4e631bc9c37..a986d603a1a 100644 --- a/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v4.rs +++ b/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v4.rs @@ -109,6 +109,7 @@ pub const DRIVE_ABCI_VALIDATION_VERSIONS_V4: DriveAbciValidationVersions = state: 0, revision: 0, transform_into_action: 0, + failed_per_transition_action: 0, data_triggers: DriveAbciValidationDataTriggerAndBindingVersions { bindings: 0, triggers: DriveAbciValidationDataTriggerVersions { diff --git a/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v5.rs b/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v5.rs index e19de80d669..bb9673de70b 100644 --- a/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v5.rs +++ b/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v5.rs @@ -110,6 +110,7 @@ pub const DRIVE_ABCI_VALIDATION_VERSIONS_V5: DriveAbciValidationVersions = state: 0, revision: 0, transform_into_action: 0, + failed_per_transition_action: 0, data_triggers: DriveAbciValidationDataTriggerAndBindingVersions { bindings: 0, triggers: DriveAbciValidationDataTriggerVersions { diff --git a/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v6.rs b/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v6.rs index bea326d225b..21838220e8f 100644 --- a/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v6.rs +++ b/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v6.rs @@ -113,6 +113,7 @@ pub const DRIVE_ABCI_VALIDATION_VERSIONS_V6: DriveAbciValidationVersions = state: 0, revision: 0, transform_into_action: 0, + failed_per_transition_action: 0, data_triggers: DriveAbciValidationDataTriggerAndBindingVersions { bindings: 0, triggers: DriveAbciValidationDataTriggerVersions { diff --git a/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v7.rs b/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v7.rs index 5549f4e7ed7..5e23882714d 100644 --- a/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v7.rs +++ b/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v7.rs @@ -107,6 +107,7 @@ pub const DRIVE_ABCI_VALIDATION_VERSIONS_V7: DriveAbciValidationVersions = state: 0, revision: 0, transform_into_action: 0, + failed_per_transition_action: 0, data_triggers: DriveAbciValidationDataTriggerAndBindingVersions { bindings: 0, triggers: DriveAbciValidationDataTriggerVersions { diff --git a/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v8.rs b/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v8.rs index db9c4505047..f7e83c01ee8 100644 --- a/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v8.rs +++ b/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v8.rs @@ -9,6 +9,16 @@ use crate::version::drive_abci_versions::drive_abci_validation_versions::{ // Bump basic_structure to v1 for contract create and update state transitions. // v1 adds config min_version enforcement: since protocol version 12, V0 config is no longer // accepted because it lacks sized_integer_types support. +// +// Issue #2867 ("validating state transition for free") is fixed at the +// aggregator layer instead — see +// `dpp_versions::dpp_validation_versions::v3::DPP_VALIDATION_VERSIONS_V3`, +// which bumps `validation_result.flatten` and `merge_many` from v0 to v1 +// for PROTOCOL_VERSION_12. This keeps the batch transformer single-version +// while changing the underlying aggregator semantics so empty-action +// failure paths become UnpaidConsensusError (tx removed from block by +// prepare_proposal) instead of being synthesised into a paid empty +// BatchTransitionAction. pub const DRIVE_ABCI_VALIDATION_VERSIONS_V8: DriveAbciValidationVersions = DriveAbciValidationVersions { state_transitions: DriveAbciStateTransitionValidationVersions { @@ -111,6 +121,13 @@ pub const DRIVE_ABCI_VALIDATION_VERSIONS_V8: DriveAbciValidationVersions = state: 0, revision: 0, transform_into_action: 0, + // PROTOCOL_VERSION_12 (v3.1 hard fork): per-transition + // failure paths in `transform_document_transition` now emit + // a `BumpIdentityDataContractNonce` action so the user pays + // for the validation work that already ran (fetch + + // ownership/revision check). v0 stays for chain + // reproducibility on PROTOCOL_VERSION_11 and below. + failed_per_transition_action: 1, data_triggers: DriveAbciValidationDataTriggerAndBindingVersions { bindings: 0, triggers: DriveAbciValidationDataTriggerVersions { diff --git a/packages/rs-platform-version/src/version/system_limits/v1.rs b/packages/rs-platform-version/src/version/system_limits/v1.rs index 3421ba58076..c4237df7ca8 100644 --- a/packages/rs-platform-version/src/version/system_limits/v1.rs +++ b/packages/rs-platform-version/src/version/system_limits/v1.rs @@ -4,6 +4,19 @@ pub const SYSTEM_LIMITS_V1: SystemLimits = SystemLimits { estimated_contract_max_serialized_size: 16384, max_field_value_size: 5120, //5 KiB max_state_transition_size: 20480, //20 KiB + // TODO: this is currently capped at 1 because the batch state-transition + // pipeline has known correctness issues with multi-transition batches: + // - It is not atomic: when one transition errors, earlier successful + // transitions inside the same batch are still applied to state. + // - Nonce-bump semantics for mixed success/failure batches are not + // well-defined: it is unclear whether to bump the nonce for the + // failed transition only, for all transitions, or for none — and the + // transformer/dispatch code does not consistently express any of + // those policies (see issue #2867). + // Before lifting this cap above 1, the whole batch validation + + // transformer + nonce-bump path must be reviewed and the atomicity / + // nonce semantics fixed. Pulling the cap higher today would expose + // those bugs to mainnet traffic. max_transitions_in_documents_batch: 1, withdrawal_transactions_per_block_limit: 4, retry_signing_expired_withdrawal_documents_per_block_limit: 1,