From 5cdc306ab65292fe69377deed151f6d403aafaf7 Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Thu, 7 May 2026 13:15:41 +0700 Subject: [PATCH 1/4] fix(drive-abci): bump nonce on failed batch document transitions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #2867. The Documents Batch transformer's per-transition handler was emitting BumpIdentityDataContractNonceAction only on `find_replaced_document_v0` failure for the Replace arm. Every other failure path (ownership-check, revision-check, plus the analogous paths in Transfer / UpdatePrice / Purchase, and Purchase's listed-price / price-mismatch checks) returned ConsensusValidationResult errors-only with no action data. The empty BatchTransitionAction that flowed up the chain still triggered fee accounting (PaidConsensusError) but the bump action's UpdateIdentityContractNonce drive op was never created — so identity_contract_nonce in state stayed at the pre-tx value. Consequence on mainnet: a user's SDK could re-broadcast the same exact serialized bytes; CheckTx FirstTimeCheck would still pass (committed state still says the nonce is unused), the proposer would include the tx in a later block, deliver_tx would fail again with the same error, and the cycle could repeat. Result: the same state-transition hash appearing in multiple blocks — exactly what mainnet ST hash 35C08D574302D32D7160E603D8159042C8606BE12FD97D952CF5FD40DB57313C did on 2026-05-04 (block 361774 idx 1 + a later block past the explorer indexer's panic point). Fix: copy the bump-emission pattern from the existing find_replaced_document_v0 failure path (lines 672-686) to all other failure sites in the Replace / Transfer / UpdatePrice / Purchase handlers. Each now returns `new_with_data_and_errors(BumpIdentityDataContractNonce(bump_action), errors)` so the contract nonce advances regardless of where the per-transition validation fails. This is a consensus-affecting change on PROTOCOL_VERSION_12 (current v3.1-dev). Validators that fail the same transition under the new behavior pay a higher fee than under the old behavior because the fee now covers the document fetch + ownership/revision check work that ran before the failure (those drive ops were already happening, just charged as 0). Three sibling tests had hard-coded fee assertions matching the old (under-charged) numbers; they're updated with comments explaining the change. Test plan - Added regression test `replayed_failed_replace_with_consumed_nonce_must_be_rejected_at_check_tx` in batch/tests/document/replacement.rs that reproduces the Techawanka.dash mainnet 35C0 shape (Create at nonce 2 → Replace at nonce 3 with mismatching revision → assert state nonce advanced AND CheckTx of identical bytes returns InvalidIdentityNonceError). RED on parent commit, GREEN with the fix. - Added decoder pin `should_decode_mainnet_35c0_documents_batch_replace_replay` in rs-dpp serialization tests confirming the mainnet wire bytes deserialize cleanly to a Batch / Replace / nonce=3 — the explorer's reported "Cannot deserialize" is its own version-skew, not a chain-bytes issue. - `cargo test -p drive-abci --lib`: 2424 passed, 0 failed (full suite). - `cargo test -p drive-abci --lib batch::tests::`: 254 passed, 0 failed. Out of scope - platform-explorer's `UNIQUE(hash)` constraint on `state_transitions` is too strict regardless of this fix; Tenderdash never guaranteed cross-block tx-hash uniqueness (failed-but-paid txs are committed by protocol design, like Ethereum reverts). Recommend `UNIQUE(block_height, hash)` upstream in pshenmic/platform-explorer. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/state_transition/serialization.rs | 70 +++++ .../batch/tests/document/nft.rs | 6 +- .../batch/tests/document/replacement.rs | 267 +++++++++++++++++- .../batch/tests/document/transfer.rs | 6 +- .../batch/transformer/v0/mod.rs | 179 +++++++++--- 5 files changed, 491 insertions(+), 37 deletions(-) diff --git a/packages/rs-dpp/src/state_transition/serialization.rs b/packages/rs-dpp/src/state_transition/serialization.rs index d02fb91d016..25d6f8b96fb 100644 --- a/packages/rs-dpp/src/state_transition/serialization.rs +++ b/packages/rs-dpp/src/state_transition/serialization.rs @@ -106,6 +106,76 @@ mod tests { assert_eq!(identity_address, EXPECTED_IDENTITY_ADDRESS); } + #[test] + /// Mainnet state transition 35C08D574302D32D7160E603D8159042C8606BE12FD97D952CF5FD40DB57313C + /// (block 361774, idx 1, status FAIL, gas 41880). + /// + /// Pinned because: + /// - Its bytes are what platform-explorer's indexer choked on with + /// `duplicate key value violates unique constraint state_transition_hash` + /// (issue #2867 mainnet escalation, 2026-05-04). The same hash appears in + /// a later block the explorer hasn't indexed yet — the second copy is + /// what triggers the unique-key panic. + /// - The chat thread misidentified it as a credit transfer; the bytes are + /// actually a single Documents Batch Replace on a DPNS-like profile doc. + /// - Identity_contract_nonce == 3, while a sibling tx in the same block + /// (5962DA04... at idx 0) succeeded with nonce == 4. So the failure path + /// is the out-of-order in-block case, not a stale replay against committed + /// state — narrowing the regression target to the failed-Replace nonce + /// bookkeeping in batch/transformer/state. + fn should_decode_mainnet_35c0_documents_batch_replace_replay() { + use crate::state_transition::batch_transition::accessors::DocumentsBatchTransitionAccessorsV0; + use crate::state_transition::batch_transition::batched_transition::document_transition::DocumentTransition; + use crate::state_transition::batch_transition::batched_transition::BatchedTransitionRef; + use crate::state_transition::StateTransitionOwned; + + const EXPECTED_STATE_TRANSITION_HASH: &str = + "35C08D574302D32D7160E603D8159042C8606BE12FD97D952CF5FD40DB57313C"; + const RAW_TRANSACTION_BASE64: &str = "AgFQIl0YZ/WZdYW/CHCAmWvwN9FYctxuUS35L5Pf73IMTgEAAQABFSmQcqs+Yj0mrj560+00qfu+k75VeNPWWC4fDlzbpAwDB3Byb2ZpbGWiobSsb+8i6ioaaOgSNkSzV4dfa0EsGBCSgcFG57JxvAADAQtkaXNwbGF5TmFtZRIKdGVjaGF3YW5rYQABQSA7dB/FyiyS6BBNBLc4x+vVMVLuy5JWjuOzOuj/4fAaLgPrC7+nQDhl4LvPx+LZ4heDNq0dQA6LF97pjACbSO0a"; + const EXPECTED_OWNER_BASE58: &str = "6Pp1RFqRnpStnY8vmp5k3ypE6rFBvzPwcoguwDXbRA7F"; + const EXPECTED_NONCE: u64 = 3; + + let raw = STANDARD + .decode(RAW_TRANSACTION_BASE64) + .expect("base64 decodes"); + let state_transition = StateTransition::deserialize_from_bytes(&raw) + .expect("dpp deserializes the wire bytes"); + + assert_eq!( + &state_transition + .transaction_id() + .expect("expected transaction id") + .encode_hex_upper::(), + EXPECTED_STATE_TRANSITION_HASH + ); + + let StateTransition::Batch(batch) = &state_transition else { + panic!("expected Batch transition, got {state_transition:?}"); + }; + + assert_eq!( + batch.owner_id().to_string(Encoding::Base58), + EXPECTED_OWNER_BASE58 + ); + + assert_eq!(batch.transitions_len(), 1); + + let only = batch + .first_transition() + .expect("expected exactly one batched transition"); + + let BatchedTransitionRef::Document(document_transition) = only else { + panic!("expected document transition, got {only:?}"); + }; + + assert!( + matches!(document_transition, DocumentTransition::Replace(_)), + "expected Replace transition, got {document_transition:?}", + ); + + assert_eq!(only.identity_contract_nonce(), EXPECTED_NONCE); + } + #[test] #[cfg(feature = "random-identities")] fn identity_create_transition_ser_de() { 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..fe4926aa6f0 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 @@ -2788,7 +2788,11 @@ mod nft_tests { assert_eq!(processing_result.valid_count(), 0); - assert_eq!(processing_result.aggregated_fees().processing_fee, 36200); + // Fee bumped from 36200 in PROTOCOL_VERSION_12 — issue #2867 fix: the + // UpdatePrice ownership-mismatch failure path now emits a bump action + // (previously dropped, leaking nonce-replay risk). Cost covers the + // fetch+validation work that was already happening. + assert_eq!(processing_result.aggregated_fees().processing_fee, 571240); let sender_documents_sql_string = format!("select * from card where $ownerId == '{}'", identity.id()); 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..25a85daa4aa 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 @@ -651,7 +651,272 @@ mod replacement_tests { assert_eq!(processing_result.valid_count(), 0); - assert_eq!(processing_result.aggregated_fees().processing_fee, 41880); + // Fee bumped from the previous 41880 (bare bump-only) to 445700 in + // PROTOCOL_VERSION_12. Issue #2867 fix: the failure path now emits the + // bump after running the document fetch + validation work, so the fee + // covers that work — same drive ops, just charged correctly. + assert_eq!(processing_result.aggregated_fees().processing_fee, 445700); + } + + /// Regression test for issue #2867 (mainnet duplicate-tx escalation, 2026-05-04). + /// + /// Mainnet ST hash 35C0...313C — a Documents Batch Replace by Techawanka.dash + /// (6Pp1RFqRnpStnY8vmp5k3ypE6rFBvzPwcoguwDXbRA7F) — landed at block 361774 idx 1 + /// as FAIL with gasUsed 41880, then re-appeared in a later block, panicking the + /// explorer indexer with `duplicate key value violates unique constraint + /// state_transition_hash`. The shape: nonce 4 ran successfully *before* nonce + /// 3 in the same block (out-of-order SDK retries), so when nonce 3's Replace + /// reached deliver_tx the doc revision had already advanced. The + /// revision-bump-by-one check in + /// batch/transformer/v0/mod.rs::check_revision_is_bumped_by_one_during_replace_v0 + /// fails, and the surrounding handler (lines 712–715) returns `Ok(result)` + /// with errors-only and **no BumpIdentityDataContractNonce action** — unlike + /// the find_replaced_document_v0 path on the same enum arm (lines 672–686) + /// which DOES emit a bump. + /// + /// Consequence: the user pays the 41880 bump fee (because the empty + /// BatchTransitionAction still triggers PaidConsensusError accounting), but + /// the contract nonce IS NEVER ADVANCED in state. The exact same bytes can + /// then be re-broadcast indefinitely; each retry lands a new failed copy in + /// a new block, all sharing the same hash. + /// + /// What this test pins: + /// 1. After a Replace that fails the revision-bump-by-one check commits, + /// the stored identity_contract_nonce MUST advance past the submitted + /// nonce. (Direct invariant — fails fast on RED.) + /// 2. Re-submitting the same exact bytes through CheckTx FirstTimeCheck + /// MUST be rejected with InvalidIdentityNonceError. (Symptom-level — + /// this is what lklimek's Feb 10 2026 testnet debug log proved was + /// broken.) + /// + /// Both assertions fail on v3.1-dev today; both should pass once the bump + /// is emitted on revision/ownership-mismatch paths in batch/transformer/v0. + #[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 a revision the chain will reject. + // Doc at revision 1 → expected revision 2 on replace. We submit + // revision 3 → check_revision_is_bumped_by_one_during_replace_v0 + // returns InvalidDocumentRevisionError(Some(1), 3). On mainnet this + // happened naturally because nonce 4 ran first and advanced the doc + // revision past what nonce 3's tx expected. + 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, + "BUG: failed Replace's bump action did not advance the contract \ + nonce. Stored nonce is still {:#x} (= post-create value), so the \ + same exact serialized bytes can be replayed forever. \ + Root cause: batch/transformer/v0/mod.rs:712-715 (and 697-700) \ + return Ok(result) with errors-only and no BumpIdentityDataContractNonce \ + action when check_revision_is_bumped_by_one_during_replace_v0 (or \ + check_ownership_of_old_replaced_document_v0) fails — unlike the \ + find_replaced_document_v0 failure path on the same arm (lines \ + 672-686) which does emit the bump.", + post_create_nonce + ); + + // 4) Symptom-level: re-submitting identical bytes through CheckTx + // FirstTimeCheck must hit the nonce check first and reject. This is + // exactly what lklimek's Feb 10 2026 testnet check_tx debug log + // showed NOT happening. + 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 — it accepted them on \ + testnet on 2026-02-10." + ); + 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] 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..232a439fdbf 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 @@ -1256,7 +1256,11 @@ mod transfer_tests { assert_eq!(processing_result.valid_count(), 0); - assert_eq!(processing_result.aggregated_fees().processing_fee, 36200); + // Fee bumped from 36200 in PROTOCOL_VERSION_12 — issue #2867 fix: the + // Transfer find-replaced-document failure path now emits a bump action + // (previously dropped, leaking nonce-replay risk). Cost covers the + // fetch+validation work that was already happening. + assert_eq!(processing_result.aggregated_fees().processing_fee, 517400); let query_sender_results = platform .drive 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..9b64e33a9e3 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 @@ -664,7 +664,7 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { Ok(document_create_action) } DocumentTransition::Replace(document_replace_transition) => { - let mut result = ConsensusValidationResult::::new(); + let result = ConsensusValidationResult::::new(); let validation_result = Self::find_replaced_document_v0(transition, replaced_documents); @@ -695,8 +695,19 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { ); if !validation_result.is_valid() { - result.merge(validation_result); - return Ok(result); + // Emit a bump action so the identity_contract_nonce advances even + // when ownership doesn't match. Without this, the same exact bytes + // could be replayed forever — see issue #2867. + let bump_action = + BumpIdentityDataContractNonceAction::from_borrowed_document_base_transition( + document_replace_transition.base(), + owner_id, + 0, + ); + return Ok(ConsensusValidationResult::new_with_data_and_errors( + BatchedTransitionAction::BumpIdentityDataContractNonce(bump_action), + validation_result.errors, + )); } if validate_against_state { @@ -710,8 +721,20 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { ); if !validation_result.is_valid() { - result.merge(validation_result); - return Ok(result); + // Emit a bump action so the identity_contract_nonce advances even + // when the revision doesn't bump by one. Without this, out-of-order + // SDK retries can replay the same bytes across multiple blocks — + // see issue #2867 (mainnet 35C0…313C, 2026-05-04). + let bump_action = + BumpIdentityDataContractNonceAction::from_borrowed_document_base_transition( + document_replace_transition.base(), + owner_id, + 0, + ); + return Ok(ConsensusValidationResult::new_with_data_and_errors( + BatchedTransitionAction::BumpIdentityDataContractNonce(bump_action), + validation_result.errors, + )); } } @@ -745,14 +768,24 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { Ok(batched_action) } DocumentTransition::Transfer(document_transfer_transition) => { - let mut result = ConsensusValidationResult::::new(); + let 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); + // Emit a bump action so the identity_contract_nonce advances even + // when the target document is missing — see issue #2867. + let bump_action = + BumpIdentityDataContractNonceAction::from_borrowed_document_base_transition( + document_transfer_transition.base(), + owner_id, + 0, + ); + return Ok(ConsensusValidationResult::new_with_data_and_errors( + BatchedTransitionAction::BumpIdentityDataContractNonce(bump_action), + validation_result.errors, + )); } let original_document = validation_result.into_data()?; @@ -764,8 +797,16 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { ); if !validation_result.is_valid() { - result.merge(validation_result); - return Ok(result); + let bump_action = + BumpIdentityDataContractNonceAction::from_borrowed_document_base_transition( + document_transfer_transition.base(), + owner_id, + 0, + ); + return Ok(ConsensusValidationResult::new_with_data_and_errors( + BatchedTransitionAction::BumpIdentityDataContractNonce(bump_action), + validation_result.errors, + )); } if validate_against_state { @@ -779,8 +820,16 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { ); if !validation_result.is_valid() { - result.merge(validation_result); - return Ok(result); + let bump_action = + BumpIdentityDataContractNonceAction::from_borrowed_document_base_transition( + document_transfer_transition.base(), + owner_id, + 0, + ); + return Ok(ConsensusValidationResult::new_with_data_and_errors( + BatchedTransitionAction::BumpIdentityDataContractNonce(bump_action), + validation_result.errors, + )); } } @@ -804,14 +853,24 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { } } DocumentTransition::UpdatePrice(document_update_price_transition) => { - let mut result = ConsensusValidationResult::::new(); + let 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); + // Emit a bump action so the identity_contract_nonce advances even + // when the target document is missing — see issue #2867. + let bump_action = + BumpIdentityDataContractNonceAction::from_borrowed_document_base_transition( + document_update_price_transition.base(), + owner_id, + 0, + ); + return Ok(ConsensusValidationResult::new_with_data_and_errors( + BatchedTransitionAction::BumpIdentityDataContractNonce(bump_action), + validation_result.errors, + )); } let original_document = validation_result.into_data()?; @@ -823,8 +882,16 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { ); if !validation_result.is_valid() { - result.merge(validation_result); - return Ok(result); + let bump_action = + BumpIdentityDataContractNonceAction::from_borrowed_document_base_transition( + document_update_price_transition.base(), + owner_id, + 0, + ); + return Ok(ConsensusValidationResult::new_with_data_and_errors( + BatchedTransitionAction::BumpIdentityDataContractNonce(bump_action), + validation_result.errors, + )); } if validate_against_state { @@ -838,8 +905,16 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { ); if !validation_result.is_valid() { - result.merge(validation_result); - return Ok(result); + let bump_action = + BumpIdentityDataContractNonceAction::from_borrowed_document_base_transition( + document_update_price_transition.base(), + owner_id, + 0, + ); + return Ok(ConsensusValidationResult::new_with_data_and_errors( + BatchedTransitionAction::BumpIdentityDataContractNonce(bump_action), + validation_result.errors, + )); } } @@ -863,14 +938,24 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { } } DocumentTransition::Purchase(document_purchase_transition) => { - let mut result = ConsensusValidationResult::::new(); + let 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); + // Emit a bump action so the identity_contract_nonce advances even + // when the target document is missing — see issue #2867. + let bump_action = + BumpIdentityDataContractNonceAction::from_borrowed_document_base_transition( + document_purchase_transition.base(), + owner_id, + 0, + ); + return Ok(ConsensusValidationResult::new_with_data_and_errors( + BatchedTransitionAction::BumpIdentityDataContractNonce(bump_action), + validation_result.errors, + )); } let original_document = validation_result.into_data()?; @@ -879,21 +964,39 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { .properties() .get_optional_integer::(PRICE)? else { - result.add_error(StateError::DocumentNotForSaleError( - DocumentNotForSaleError::new(original_document.id()), + let bump_action = + BumpIdentityDataContractNonceAction::from_borrowed_document_base_transition( + document_purchase_transition.base(), + owner_id, + 0, + ); + return Ok(ConsensusValidationResult::new_with_data_and_errors( + BatchedTransitionAction::BumpIdentityDataContractNonce(bump_action), + vec![StateError::DocumentNotForSaleError( + DocumentNotForSaleError::new(original_document.id()), + ) + .into()], )); - return Ok(result); }; if listed_price != document_purchase_transition.price() { - result.add_error(StateError::DocumentIncorrectPurchasePriceError( - DocumentIncorrectPurchasePriceError::new( - original_document.id(), - document_purchase_transition.price(), - listed_price, - ), + let bump_action = + BumpIdentityDataContractNonceAction::from_borrowed_document_base_transition( + document_purchase_transition.base(), + owner_id, + 0, + ); + return Ok(ConsensusValidationResult::new_with_data_and_errors( + BatchedTransitionAction::BumpIdentityDataContractNonce(bump_action), + vec![StateError::DocumentIncorrectPurchasePriceError( + DocumentIncorrectPurchasePriceError::new( + original_document.id(), + document_purchase_transition.price(), + listed_price, + ), + ) + .into()], )); - return Ok(result); } if validate_against_state { @@ -907,8 +1010,16 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { ); if !validation_result.is_valid() { - result.merge(validation_result); - return Ok(result); + let bump_action = + BumpIdentityDataContractNonceAction::from_borrowed_document_base_transition( + document_purchase_transition.base(), + owner_id, + 0, + ); + return Ok(ConsensusValidationResult::new_with_data_and_errors( + BatchedTransitionAction::BumpIdentityDataContractNonce(bump_action), + validation_result.errors, + )); } } From 1d0a02bc3e9efd713f273293946456d285075417 Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Fri, 8 May 2026 02:15:00 +0700 Subject: [PATCH 2/4] refactor(drive-abci): tighten bump-emission framing in batch transformer + tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reframe the change from a mainnet duplicate-tx narrative to its actual purpose: the user pays for the per-transition validation work that ran before a failure (document fetch + ownership / revision check), instead of getting it for free because the failure path emitted no action. - Drop the dead `let result = ConsensusValidationResult::new();` and `if result.is_valid() { ... } else { Ok(result) }` tails in Replace, Transfer, UpdatePrice and Purchase arms — `result` is never modified after the per-path failure handlers were rewritten to return early. - One short doc on `transform_document_transition_v0` explaining why every per-transition failure emits a bump (charge for the work, advance the nonce); strip per-site narrative comments that pointed at issue #2867 / mainnet 35C0…313C. - Slim the regression test doc: keep what it pins, drop the mainnet escalation timeline. - Slim the fee-change comments in replacement / nft / transfer to a one-liner ("covers the fetch + check that ran before the failure"). - Drop the mainnet 35C0 decoder pin in `packages/rs-dpp/src/state_transition/serialization.rs` — it was a one-off debug artifact, not a behavior pin for this PR. The architectural mainnet fix (return `data: None` from aggregators so empty-action batches become UnpaidConsensusError → tx removed from block) lives in PR #3616. This PR is the complementary "user pays for advanced-structure validation work" layer for paths that DO produce a real per-transition error inside an otherwise-valid batch. --- .../src/state_transition/serialization.rs | 70 ---------------- .../batch/tests/document/nft.rs | 7 +- .../batch/tests/document/replacement.rs | 81 ++++++------------- .../batch/tests/document/transfer.rs | 7 +- .../batch/transformer/v0/mod.rs | 65 ++++----------- 5 files changed, 45 insertions(+), 185 deletions(-) diff --git a/packages/rs-dpp/src/state_transition/serialization.rs b/packages/rs-dpp/src/state_transition/serialization.rs index 25d6f8b96fb..d02fb91d016 100644 --- a/packages/rs-dpp/src/state_transition/serialization.rs +++ b/packages/rs-dpp/src/state_transition/serialization.rs @@ -106,76 +106,6 @@ mod tests { assert_eq!(identity_address, EXPECTED_IDENTITY_ADDRESS); } - #[test] - /// Mainnet state transition 35C08D574302D32D7160E603D8159042C8606BE12FD97D952CF5FD40DB57313C - /// (block 361774, idx 1, status FAIL, gas 41880). - /// - /// Pinned because: - /// - Its bytes are what platform-explorer's indexer choked on with - /// `duplicate key value violates unique constraint state_transition_hash` - /// (issue #2867 mainnet escalation, 2026-05-04). The same hash appears in - /// a later block the explorer hasn't indexed yet — the second copy is - /// what triggers the unique-key panic. - /// - The chat thread misidentified it as a credit transfer; the bytes are - /// actually a single Documents Batch Replace on a DPNS-like profile doc. - /// - Identity_contract_nonce == 3, while a sibling tx in the same block - /// (5962DA04... at idx 0) succeeded with nonce == 4. So the failure path - /// is the out-of-order in-block case, not a stale replay against committed - /// state — narrowing the regression target to the failed-Replace nonce - /// bookkeeping in batch/transformer/state. - fn should_decode_mainnet_35c0_documents_batch_replace_replay() { - use crate::state_transition::batch_transition::accessors::DocumentsBatchTransitionAccessorsV0; - use crate::state_transition::batch_transition::batched_transition::document_transition::DocumentTransition; - use crate::state_transition::batch_transition::batched_transition::BatchedTransitionRef; - use crate::state_transition::StateTransitionOwned; - - const EXPECTED_STATE_TRANSITION_HASH: &str = - "35C08D574302D32D7160E603D8159042C8606BE12FD97D952CF5FD40DB57313C"; - const RAW_TRANSACTION_BASE64: &str = "AgFQIl0YZ/WZdYW/CHCAmWvwN9FYctxuUS35L5Pf73IMTgEAAQABFSmQcqs+Yj0mrj560+00qfu+k75VeNPWWC4fDlzbpAwDB3Byb2ZpbGWiobSsb+8i6ioaaOgSNkSzV4dfa0EsGBCSgcFG57JxvAADAQtkaXNwbGF5TmFtZRIKdGVjaGF3YW5rYQABQSA7dB/FyiyS6BBNBLc4x+vVMVLuy5JWjuOzOuj/4fAaLgPrC7+nQDhl4LvPx+LZ4heDNq0dQA6LF97pjACbSO0a"; - const EXPECTED_OWNER_BASE58: &str = "6Pp1RFqRnpStnY8vmp5k3ypE6rFBvzPwcoguwDXbRA7F"; - const EXPECTED_NONCE: u64 = 3; - - let raw = STANDARD - .decode(RAW_TRANSACTION_BASE64) - .expect("base64 decodes"); - let state_transition = StateTransition::deserialize_from_bytes(&raw) - .expect("dpp deserializes the wire bytes"); - - assert_eq!( - &state_transition - .transaction_id() - .expect("expected transaction id") - .encode_hex_upper::(), - EXPECTED_STATE_TRANSITION_HASH - ); - - let StateTransition::Batch(batch) = &state_transition else { - panic!("expected Batch transition, got {state_transition:?}"); - }; - - assert_eq!( - batch.owner_id().to_string(Encoding::Base58), - EXPECTED_OWNER_BASE58 - ); - - assert_eq!(batch.transitions_len(), 1); - - let only = batch - .first_transition() - .expect("expected exactly one batched transition"); - - let BatchedTransitionRef::Document(document_transition) = only else { - panic!("expected document transition, got {only:?}"); - }; - - assert!( - matches!(document_transition, DocumentTransition::Replace(_)), - "expected Replace transition, got {document_transition:?}", - ); - - assert_eq!(only.identity_contract_nonce(), EXPECTED_NONCE); - } - #[test] #[cfg(feature = "random-identities")] fn identity_create_transition_ser_de() { 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 fe4926aa6f0..e95b680df29 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 @@ -2788,10 +2788,9 @@ mod nft_tests { assert_eq!(processing_result.valid_count(), 0); - // Fee bumped from 36200 in PROTOCOL_VERSION_12 — issue #2867 fix: the - // UpdatePrice ownership-mismatch failure path now emits a bump action - // (previously dropped, leaking nonce-replay risk). Cost covers the - // fetch+validation work that was already happening. + // Covers the fetch + ownership check that ran before the failure. + // Previously the failure path emitted no action and this work was + // charged as 0. assert_eq!(processing_result.aggregated_fees().processing_fee, 571240); let sender_documents_sql_string = 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 25a85daa4aa..f3ce04b4937 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 @@ -651,46 +651,24 @@ mod replacement_tests { assert_eq!(processing_result.valid_count(), 0); - // Fee bumped from the previous 41880 (bare bump-only) to 445700 in - // PROTOCOL_VERSION_12. Issue #2867 fix: the failure path now emits the - // bump after running the document fetch + validation work, so the fee - // covers that work — same drive ops, just charged correctly. + // Covers the document fetch + structure validation that ran before + // the failure. Previously the failure path emitted no action and this + // work was charged as 0; now the bump covers it. assert_eq!(processing_result.aggregated_fees().processing_fee, 445700); } - /// Regression test for issue #2867 (mainnet duplicate-tx escalation, 2026-05-04). + /// Pins the bump-emission contract on Replace's revision-mismatch path. /// - /// Mainnet ST hash 35C0...313C — a Documents Batch Replace by Techawanka.dash - /// (6Pp1RFqRnpStnY8vmp5k3ypE6rFBvzPwcoguwDXbRA7F) — landed at block 361774 idx 1 - /// as FAIL with gasUsed 41880, then re-appeared in a later block, panicking the - /// explorer indexer with `duplicate key value violates unique constraint - /// state_transition_hash`. The shape: nonce 4 ran successfully *before* nonce - /// 3 in the same block (out-of-order SDK retries), so when nonce 3's Replace - /// reached deliver_tx the doc revision had already advanced. The - /// revision-bump-by-one check in - /// batch/transformer/v0/mod.rs::check_revision_is_bumped_by_one_during_replace_v0 - /// fails, and the surrounding handler (lines 712–715) returns `Ok(result)` - /// with errors-only and **no BumpIdentityDataContractNonce action** — unlike - /// the find_replaced_document_v0 path on the same enum arm (lines 672–686) - /// which DOES emit a bump. + /// 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. /// - /// Consequence: the user pays the 41880 bump fee (because the empty - /// BatchTransitionAction still triggers PaidConsensusError accounting), but - /// the contract nonce IS NEVER ADVANCED in state. The exact same bytes can - /// then be re-broadcast indefinitely; each retry lands a new failed copy in - /// a new block, all sharing the same hash. - /// - /// What this test pins: - /// 1. After a Replace that fails the revision-bump-by-one check commits, - /// the stored identity_contract_nonce MUST advance past the submitted - /// nonce. (Direct invariant — fails fast on RED.) - /// 2. Re-submitting the same exact bytes through CheckTx FirstTimeCheck - /// MUST be rejected with InvalidIdentityNonceError. (Symptom-level — - /// this is what lklimek's Feb 10 2026 testnet debug log proved was - /// broken.) - /// - /// Both assertions fail on v3.1-dev today; both should pass once the bump - /// is emitted on revision/ownership-mismatch paths in batch/transformer/v0. + /// 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; @@ -792,12 +770,10 @@ mod replacement_tests { let post_create_nonce = post_create_nonce_raw.expect("contract nonce must be present after create"); - // 2) Build a Replace at nonce 3 with a revision the chain will reject. - // Doc at revision 1 → expected revision 2 on replace. We submit - // revision 3 → check_revision_is_bumped_by_one_during_replace_v0 - // returns InvalidDocumentRevisionError(Some(1), 3). On mainnet this - // happened naturally because nonce 4 ran first and advanced the doc - // revision past what nonce 3's tx expected. + // 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()); @@ -867,22 +843,14 @@ mod replacement_tests { assert_ne!( post_replace_nonce, post_create_nonce, - "BUG: failed Replace's bump action did not advance the contract \ - nonce. Stored nonce is still {:#x} (= post-create value), so the \ - same exact serialized bytes can be replayed forever. \ - Root cause: batch/transformer/v0/mod.rs:712-715 (and 697-700) \ - return Ok(result) with errors-only and no BumpIdentityDataContractNonce \ - action when check_revision_is_bumped_by_one_during_replace_v0 (or \ - check_ownership_of_old_replaced_document_v0) fails — unlike the \ - find_replaced_document_v0 failure path on the same arm (lines \ - 672-686) which does emit the bump.", + "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) Symptom-level: re-submitting identical bytes through CheckTx - // FirstTimeCheck must hit the nonce check first and reject. This is - // exactly what lklimek's Feb 10 2026 testnet check_tx debug log - // showed NOT happening. + // 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"); @@ -905,9 +873,8 @@ mod replacement_tests { assert!( !check_tx_result.is_valid(), - "CheckTx FirstTimeCheck MUST reject identical bytes after the \ - failed-Replace bump consumed the nonce — it accepted them on \ - testnet on 2026-02-10." + "CheckTx FirstTimeCheck must reject identical bytes after the \ + failed-Replace bump consumed the nonce" ); assert!( check_tx_result.errors.iter().any(|e| matches!( 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 232a439fdbf..c318a921ad7 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 @@ -1256,10 +1256,9 @@ mod transfer_tests { assert_eq!(processing_result.valid_count(), 0); - // Fee bumped from 36200 in PROTOCOL_VERSION_12 — issue #2867 fix: the - // Transfer find-replaced-document failure path now emits a bump action - // (previously dropped, leaking nonce-replay risk). Cost covers the - // fetch+validation work that was already happening. + // Covers the fetch + ownership check that ran before the failure. + // Previously the failure path emitted no action and this work was + // charged as 0. assert_eq!(processing_result.aggregated_fees().processing_fee, 517400); let query_sender_results = platform 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 9b64e33a9e3..fd1e5edaec1 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 @@ -637,7 +637,14 @@ 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.) emits a `BumpIdentityDataContractNonce` action so the user pays + /// for the validation work that already ran (fetch + ownership/revision + /// checks) and the contract nonce advances. Without this, the failure + /// path would return errors-only with no action data, fee accounting + /// would charge 0, and the same nonce would remain available — i.e. a + /// "free advanced-structure validation" hole. fn transform_document_transition_v0<'a>( drive: &Drive, transaction: TransactionArg, @@ -664,24 +671,19 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { Ok(document_create_action) } DocumentTransition::Replace(document_replace_transition) => { - let 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 + // user_fee_increase is set on the outer Documents Batch 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,9 +697,6 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { ); if !validation_result.is_valid() { - // Emit a bump action so the identity_contract_nonce advances even - // when ownership doesn't match. Without this, the same exact bytes - // could be replayed forever — see issue #2867. let bump_action = BumpIdentityDataContractNonceAction::from_borrowed_document_base_transition( document_replace_transition.base(), @@ -711,9 +710,7 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { } if validate_against_state { - //there are situations where we don't want to validate this against the state - // for example when we already applied the state transition action - // and we are just validating it happened + // Skipped on the rerun path where the action has already been applied. let validation_result = Self::check_revision_is_bumped_by_one_during_replace_v0( document_replace_transition.revision(), document_replace_transition.base().id(), @@ -721,10 +718,6 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { ); if !validation_result.is_valid() { - // Emit a bump action so the identity_contract_nonce advances even - // when the revision doesn't bump by one. Without this, out-of-order - // SDK retries can replay the same bytes across multiple blocks — - // see issue #2867 (mainnet 35C0…313C, 2026-05-04). let bump_action = BumpIdentityDataContractNonceAction::from_borrowed_document_base_transition( document_replace_transition.base(), @@ -751,11 +744,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| { @@ -768,14 +757,10 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { Ok(batched_action) } DocumentTransition::Transfer(document_transfer_transition) => { - let result = ConsensusValidationResult::::new(); - let validation_result = Self::find_replaced_document_v0(transition, replaced_documents); if !validation_result.is_valid_with_data() { - // Emit a bump action so the identity_contract_nonce advances even - // when the target document is missing — see issue #2867. let bump_action = BumpIdentityDataContractNonceAction::from_borrowed_document_base_transition( document_transfer_transition.base(), @@ -846,21 +831,13 @@ 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 result = ConsensusValidationResult::::new(); - let validation_result = Self::find_replaced_document_v0(transition, replaced_documents); if !validation_result.is_valid_with_data() { - // Emit a bump action so the identity_contract_nonce advances even - // when the target document is missing — see issue #2867. let bump_action = BumpIdentityDataContractNonceAction::from_borrowed_document_base_transition( document_update_price_transition.base(), @@ -931,21 +908,13 @@ 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 result = ConsensusValidationResult::::new(); - let validation_result = Self::find_replaced_document_v0(transition, replaced_documents); if !validation_result.is_valid_with_data() { - // Emit a bump action so the identity_contract_nonce advances even - // when the target document is missing — see issue #2867. let bump_action = BumpIdentityDataContractNonceAction::from_borrowed_document_base_transition( document_purchase_transition.base(), @@ -1037,11 +1006,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) } } } From aa47a8289d115d735909b01d035c05dd93d631b1 Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Fri, 8 May 2026 02:24:12 +0700 Subject: [PATCH 3/4] style(drive-abci): cargo fmt for #2867 bump-emission cleanup --- .../batch/tests/document/replacement.rs | 27 ++++++++++--------- .../batch/transformer/v0/mod.rs | 10 ++++--- 2 files changed, 20 insertions(+), 17 deletions(-) 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 f3ce04b4937..75a4182d934 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 @@ -778,19 +778,20 @@ mod replacement_tests { 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_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() 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 fd1e5edaec1..6626e1fb39f 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 @@ -941,10 +941,12 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { ); return Ok(ConsensusValidationResult::new_with_data_and_errors( BatchedTransitionAction::BumpIdentityDataContractNonce(bump_action), - vec![StateError::DocumentNotForSaleError( - DocumentNotForSaleError::new(original_document.id()), - ) - .into()], + vec![ + StateError::DocumentNotForSaleError(DocumentNotForSaleError::new( + original_document.id(), + )) + .into(), + ], )); }; From be4a538372563e2ed75cefbe7f4129675b0b03ad Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Fri, 8 May 2026 02:38:43 +0700 Subject: [PATCH 4/4] fix(drive-abci): version-gate per-transition bump emission, add v11 paired tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address PR #3608 review comments from QuantumExplorer. - The previous commit emitted bump actions on every per-transition failure unconditionally, which would diverge PROTOCOL_VERSION_11 chain replay (the user-paid fees and stored contract nonce both shifted). Gate the bump emission on a new `failed_per_transition_action: FeatureVersion` field on `DriveAbciDocumentsStateTransitionValidationVersions`: - v0 (PROTOCOL_VERSION_11 and below): errors-only, no action data — preserves chain history bit-for-bit. - v1 (PROTOCOL_VERSION_12+, set in DRIVE_ABCI_VALIDATION_VERSIONS_V8): emit `BumpIdentityDataContractNonce` action. - Factor the 13 per-transition failure sites in `transform_document_transition` into a single `Self::failed_per_transition_action(...)` helper that dispatches on the new field. - Add a function-level doc on `transform_document_transition_v0` noting that the `0` user_fee_increase passed into each `BumpIdentityDataContractNonceAction::from_borrowed_document_base_transition` call is 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. - Refactor three sibling tests into paired-version tests so the v11 baseline fees stay pinned alongside the v12 post-fix fees: - test_document_replace_on_document_type_that_is_not_mutable + _protocol_version_11 (445700 / 41880) - test_document_transfer_that_does_not_yet_exist + _protocol_version_11 (517400 / 36200) - test_document_set_price_on_not_owned_document + _protocol_version_11 (571240 / 36200) --- .../batch/tests/document/nft.rs | 41 ++- .../batch/tests/document/replacement.rs | 44 +++- .../batch/tests/document/transfer.rs | 41 ++- .../batch/transformer/v0/mod.rs | 249 +++++++++--------- .../drive_abci_validation_versions/mod.rs | 15 ++ .../drive_abci_validation_versions/v1.rs | 1 + .../drive_abci_validation_versions/v2.rs | 1 + .../drive_abci_validation_versions/v3.rs | 1 + .../drive_abci_validation_versions/v4.rs | 1 + .../drive_abci_validation_versions/v5.rs | 1 + .../drive_abci_validation_versions/v6.rs | 1 + .../drive_abci_validation_versions/v7.rs | 1 + .../drive_abci_validation_versions/v8.rs | 7 + 13 files changed, 255 insertions(+), 149 deletions(-) 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 e95b680df29..7ee10c09c86 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 @@ -2652,10 +2652,17 @@ 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. Same + /// scenario at PROTOCOL_VERSION_11 (legacy bump-only fee) and + /// PROTOCOL_VERSION_12 (fee covers fetch + validation work). + 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); @@ -2788,10 +2795,12 @@ mod nft_tests { assert_eq!(processing_result.valid_count(), 0); - // Covers the fetch + ownership check that ran before the failure. - // Previously the failure path emitted no action and this work was - // charged as 0. - assert_eq!(processing_result.aggregated_fees().processing_fee, 571240); + 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()); @@ -2821,6 +2830,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 75a4182d934..a4faef44229 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,10 +657,32 @@ mod replacement_tests { assert_eq!(processing_result.valid_count(), 0); - // Covers the document fetch + structure validation that ran before - // the failure. Previously the failure path emitted no action and this - // work was charged as 0; now the bump covers it. - assert_eq!(processing_result.aggregated_fees().processing_fee, 445700); + 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. 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 c318a921ad7..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,10 +1263,12 @@ mod transfer_tests { assert_eq!(processing_result.valid_count(), 0); - // Covers the fetch + ownership check that ran before the failure. - // Previously the failure path emitted no action and this work was - // charged as 0. - assert_eq!(processing_result.aggregated_fees().processing_fee, 517400); + 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 @@ -1277,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/transformer/v0/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/transformer/v0/mod.rs index 6626e1fb39f..c2803b7e5a5 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 @@ -166,6 +166,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 { @@ -645,6 +651,13 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { /// path would return errors-only with no action data, fee accounting /// would charge 0, and the same nonce would remain available — i.e. a /// "free advanced-structure validation" hole. + /// + /// 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, @@ -675,17 +688,12 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { Self::find_replaced_document_v0(transition, replaced_documents); if !validation_result.is_valid_with_data() { - // user_fee_increase is set on the outer Documents Batch - let bump_action = - BumpIdentityDataContractNonceAction::from_borrowed_document_base_transition( - document_replace_transition.base(), - owner_id, - 0, - ); - return Ok(ConsensusValidationResult::new_with_data_and_errors( - BatchedTransitionAction::BumpIdentityDataContractNonce(bump_action), + return Self::failed_per_transition_action( + document_replace_transition.base(), + owner_id, validation_result.errors, - )); + platform_version, + ); } let original_document = validation_result.into_data()?; @@ -697,16 +705,12 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { ); if !validation_result.is_valid() { - let bump_action = - BumpIdentityDataContractNonceAction::from_borrowed_document_base_transition( - document_replace_transition.base(), - owner_id, - 0, - ); - return Ok(ConsensusValidationResult::new_with_data_and_errors( - BatchedTransitionAction::BumpIdentityDataContractNonce(bump_action), + return Self::failed_per_transition_action( + document_replace_transition.base(), + owner_id, validation_result.errors, - )); + platform_version, + ); } if validate_against_state { @@ -718,16 +722,12 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { ); if !validation_result.is_valid() { - let bump_action = - BumpIdentityDataContractNonceAction::from_borrowed_document_base_transition( - document_replace_transition.base(), - owner_id, - 0, - ); - return Ok(ConsensusValidationResult::new_with_data_and_errors( - BatchedTransitionAction::BumpIdentityDataContractNonce(bump_action), + return Self::failed_per_transition_action( + document_replace_transition.base(), + owner_id, validation_result.errors, - )); + platform_version, + ); } } @@ -761,16 +761,12 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { Self::find_replaced_document_v0(transition, replaced_documents); if !validation_result.is_valid_with_data() { - let bump_action = - BumpIdentityDataContractNonceAction::from_borrowed_document_base_transition( - document_transfer_transition.base(), - owner_id, - 0, - ); - return Ok(ConsensusValidationResult::new_with_data_and_errors( - BatchedTransitionAction::BumpIdentityDataContractNonce(bump_action), + return Self::failed_per_transition_action( + document_transfer_transition.base(), + owner_id, validation_result.errors, - )); + platform_version, + ); } let original_document = validation_result.into_data()?; @@ -782,22 +778,16 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { ); if !validation_result.is_valid() { - let bump_action = - BumpIdentityDataContractNonceAction::from_borrowed_document_base_transition( - document_transfer_transition.base(), - owner_id, - 0, - ); - return Ok(ConsensusValidationResult::new_with_data_and_errors( - BatchedTransitionAction::BumpIdentityDataContractNonce(bump_action), + return Self::failed_per_transition_action( + document_transfer_transition.base(), + owner_id, validation_result.errors, - )); + platform_version, + ); } if validate_against_state { - //there are situations where we don't want to validate this against the state - // for example when we already applied the state transition action - // and we are just validating it happened + // Skipped on the rerun path where the action has already been applied. let validation_result = Self::check_revision_is_bumped_by_one_during_replace_v0( document_transfer_transition.revision(), document_transfer_transition.base().id(), @@ -805,16 +795,12 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { ); if !validation_result.is_valid() { - let bump_action = - BumpIdentityDataContractNonceAction::from_borrowed_document_base_transition( - document_transfer_transition.base(), - owner_id, - 0, - ); - return Ok(ConsensusValidationResult::new_with_data_and_errors( - BatchedTransitionAction::BumpIdentityDataContractNonce(bump_action), + return Self::failed_per_transition_action( + document_transfer_transition.base(), + owner_id, validation_result.errors, - )); + platform_version, + ); } } @@ -838,16 +824,12 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { Self::find_replaced_document_v0(transition, replaced_documents); if !validation_result.is_valid_with_data() { - let bump_action = - BumpIdentityDataContractNonceAction::from_borrowed_document_base_transition( - document_update_price_transition.base(), - owner_id, - 0, - ); - return Ok(ConsensusValidationResult::new_with_data_and_errors( - BatchedTransitionAction::BumpIdentityDataContractNonce(bump_action), + 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()?; @@ -859,22 +841,16 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { ); if !validation_result.is_valid() { - let bump_action = - BumpIdentityDataContractNonceAction::from_borrowed_document_base_transition( - document_update_price_transition.base(), - owner_id, - 0, - ); - return Ok(ConsensusValidationResult::new_with_data_and_errors( - BatchedTransitionAction::BumpIdentityDataContractNonce(bump_action), + return Self::failed_per_transition_action( + document_update_price_transition.base(), + owner_id, validation_result.errors, - )); + platform_version, + ); } if validate_against_state { - //there are situations where we don't want to validate this against the state - // for example when we already applied the state transition action - // and we are just validating it happened + // Skipped on the rerun path where the action has already been applied. let validation_result = Self::check_revision_is_bumped_by_one_during_replace_v0( document_update_price_transition.revision(), document_update_price_transition.base().id(), @@ -882,16 +858,12 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { ); if !validation_result.is_valid() { - let bump_action = - BumpIdentityDataContractNonceAction::from_borrowed_document_base_transition( - document_update_price_transition.base(), - owner_id, - 0, - ); - return Ok(ConsensusValidationResult::new_with_data_and_errors( - BatchedTransitionAction::BumpIdentityDataContractNonce(bump_action), + return Self::failed_per_transition_action( + document_update_price_transition.base(), + owner_id, validation_result.errors, - )); + platform_version, + ); } } @@ -915,16 +887,12 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { Self::find_replaced_document_v0(transition, replaced_documents); if !validation_result.is_valid_with_data() { - let bump_action = - BumpIdentityDataContractNonceAction::from_borrowed_document_base_transition( - document_purchase_transition.base(), - owner_id, - 0, - ); - return Ok(ConsensusValidationResult::new_with_data_and_errors( - BatchedTransitionAction::BumpIdentityDataContractNonce(bump_action), + return Self::failed_per_transition_action( + document_purchase_transition.base(), + owner_id, validation_result.errors, - )); + platform_version, + ); } let original_document = validation_result.into_data()?; @@ -933,32 +901,23 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { .properties() .get_optional_integer::(PRICE)? else { - let bump_action = - BumpIdentityDataContractNonceAction::from_borrowed_document_base_transition( - document_purchase_transition.base(), - owner_id, - 0, - ); - return Ok(ConsensusValidationResult::new_with_data_and_errors( - BatchedTransitionAction::BumpIdentityDataContractNonce(bump_action), + 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() { - let bump_action = - BumpIdentityDataContractNonceAction::from_borrowed_document_base_transition( - document_purchase_transition.base(), - owner_id, - 0, - ); - return Ok(ConsensusValidationResult::new_with_data_and_errors( - BatchedTransitionAction::BumpIdentityDataContractNonce(bump_action), + return Self::failed_per_transition_action( + document_purchase_transition.base(), + owner_id, vec![StateError::DocumentIncorrectPurchasePriceError( DocumentIncorrectPurchasePriceError::new( original_document.id(), @@ -967,13 +926,12 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { ), ) .into()], - )); + platform_version, + ); } if validate_against_state { - //there are situations where we don't want to validate this against the state - // for example when we already applied the state transition action - // and we are just validating it happened + // Skipped on the rerun path where the action has already been applied. let validation_result = Self::check_revision_is_bumped_by_one_during_replace_v0( document_purchase_transition.revision(), document_purchase_transition.base().id(), @@ -981,16 +939,12 @@ impl BatchTransitionInternalTransformerV0 for BatchTransition { ); if !validation_result.is_valid() { - let bump_action = - BumpIdentityDataContractNonceAction::from_borrowed_document_base_transition( - document_purchase_transition.base(), - owner_id, - 0, - ); - return Ok(ConsensusValidationResult::new_with_data_and_errors( - BatchedTransitionAction::BumpIdentityDataContractNonce(bump_action), + return Self::failed_per_transition_action( + document_purchase_transition.base(), + owner_id, validation_result.errors, - )); + platform_version, + ); } } @@ -1081,4 +1035,45 @@ 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. + // The `0` user_fee_increase here is overridden by the outer + // Documents Batch when this per-transition action rolls up. + 1 => { + 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/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..03fbc32d043 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 @@ -111,6 +111,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 {