From 1aecd578afdbee480c9c4f66ec9c362286a22271 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Mon, 17 Aug 2026 18:22:05 +0700 Subject: [PATCH 1/4] fix(drive-abci): roll back dropped state transitions on the proposing path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hotfix for the mainnet evo1 stalls of 2026-08-14/15 (after heights 415652 and 415661). Execution can write into the shared block transaction before failing — the address-input fee flow is apply-then-check, and the estimated fee used for admission can undershoot the actual metered fee for a Shield (dashpay/grovedb#812) — so a transition dropped as InternalError left its writes in the transaction. The proposer then gossiped a block WITHOUT the transition (TxAction::Removed) while advertising an app hash computed WITH its writes. No validator could reproduce that hash, and every proposer whose mempool carried the transition burned its round: the chain stalled for a full proposer rotation (~2h at quorum size 100), and the trigger is remotely repeatable by anyone at the cost of one Unshield. When building a proposal, wrap each executed state transition in a GroveDB savepoint and roll back if its result strips it from the block (InternalError or UnpaidConsensusError). The proposal then omits the transition AND its app hash omits its writes, so any validator — including un-upgraded v4.1.0 ones — reproduces the hash and the round commits. This is deliberately proposer-side only and consensus-invisible, so it needs no protocol-version gate and protects incrementally as masternodes upgrade: each upgraded proposer immediately stops poisoning its own proposals, and a stall triggered mid-rollout ends at the first upgraded proposer's slot instead of running a full rotation. The validation path is untouched — rolling back there would change what state a received block evaluates to, a consensus change that rides the protocol v14 gate instead (dashpay/platform#4408). The test validating_must_behave_exactly_as_v4_1_0 pins that path bit-for-bit, leak included, and fails if this hotfix ever silently becomes a fork. Savepoints of kept transitions stay on the stack (RocksDB exposes no pop-without-rollback) and die with the per-round transaction; the genesis re-proposal path (prepare_proposal, process_proposal, mimic) now drains the stack instead of popping once so the residue cannot redirect it. Co-Authored-By: Claude Fable 5 --- .../src/abci/handler/prepare_proposal.rs | 7 + .../src/abci/handler/process_proposal.rs | 7 + .../state_transition_processing/mod.rs | 3 + .../process_raw_state_transitions/mod.rs | 3 + .../process_raw_state_transitions/v0/mod.rs | 79 ++++++ .../state_transitions/shield/tests.rs | 259 ++++++++++++++++++ packages/rs-drive-abci/src/mimic/mod.rs | 5 + 7 files changed, 363 insertions(+) diff --git a/packages/rs-drive-abci/src/abci/handler/prepare_proposal.rs b/packages/rs-drive-abci/src/abci/handler/prepare_proposal.rs index 10a4bd95631..500f9484e30 100644 --- a/packages/rs-drive-abci/src/abci/handler/prepare_proposal.rs +++ b/packages/rs-drive-abci/src/abci/handler/prepare_proposal.rs @@ -143,6 +143,13 @@ where if let Some(tx) = transaction_guard.as_ref() { tx.rollback_to_savepoint() .map_err(|e| drive::grovedb::error::Error::StorageError(RocksDBError(e)))?; + // Drain the rest of the savepoint stack: the state-transition loop leaves one + // savepoint per transition executed while proposing (see the proposer-side + // rollback in process_raw_state_transitions_v0), so a single rollback may only + // rewind to the last transition of the previous round. Every savepoint on this + // stack records the post-init-chain state or later, and the bottom one records + // exactly it, so draining until empty always lands on the post-init-chain state. + while tx.rollback_to_savepoint().is_ok() {} tx.set_savepoint(); } transaction_guard diff --git a/packages/rs-drive-abci/src/abci/handler/process_proposal.rs b/packages/rs-drive-abci/src/abci/handler/process_proposal.rs index a64aba5013b..cb3c41cc79a 100644 --- a/packages/rs-drive-abci/src/abci/handler/process_proposal.rs +++ b/packages/rs-drive-abci/src/abci/handler/process_proposal.rs @@ -174,6 +174,13 @@ where if let Some(tx) = transaction_guard.as_ref() { tx.rollback_to_savepoint() .map_err(|e| drive::grovedb::error::Error::StorageError(RocksDBError(e)))?; + // Drain the rest of the savepoint stack: the state-transition loop leaves one + // savepoint per transition executed while proposing (see the proposer-side + // rollback in process_raw_state_transitions_v0), so a single rollback may only + // rewind to the last transition of the previous round. Every savepoint on this + // stack records the post-init-chain state or later, and the bottom one records + // exactly it, so draining until empty always lands on the post-init-chain state. + while tx.rollback_to_savepoint().is_ok() {} tx.set_savepoint(); } transaction_guard diff --git a/packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/mod.rs index 82d014283d2..e7ad6d90e97 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/mod.rs @@ -3,6 +3,9 @@ mod decode_raw_state_transitions; mod execute_event; mod process_raw_state_transitions; mod process_validation_result; + +#[cfg(test)] +pub(crate) use process_raw_state_transitions::test_fault_injection; mod record_added_balance_outputs; mod store_address_balances_to_recent_block_storage; mod validate_fees_of_event; diff --git a/packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/process_raw_state_transitions/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/process_raw_state_transitions/mod.rs index 70a5d0155a0..77b47f89f03 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/process_raw_state_transitions/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/process_raw_state_transitions/mod.rs @@ -1,5 +1,8 @@ mod v0; +#[cfg(test)] +pub(crate) use v0::test_fault_injection; + use crate::error::execution::ExecutionError; use crate::error::Error; use crate::metrics::HistogramTiming; diff --git a/packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/process_raw_state_transitions/v0/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/process_raw_state_transitions/v0/mod.rs index d8596ae2eb9..3990ae631bb 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/process_raw_state_transitions/v0/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/process_raw_state_transitions/v0/mod.rs @@ -17,10 +17,25 @@ use crate::platform_types::state_transitions_processing_result::{ use dpp::util::hash::hash_single; use dpp::version::PlatformVersion; use drive::grovedb::Transaction; +use drive::grovedb_storage::Error::RocksDBError; use std::time::Instant; use super::super::StateTransitionAwareError; +/// Test-only fault injection: force the next successfully executed state transition to be +/// reported as an `InternalError` AFTER its drive operations were applied. This models the +/// only way an `InternalError` can carry state (an `Err` surfacing after +/// `apply_drive_operations(apply = true)`, e.g. the address-input fee coverage guard failing +/// on an under-estimated `Shield`) without depending on any particular estimation bug. +#[cfg(test)] +pub(crate) mod test_fault_injection { + use std::cell::Cell; + + thread_local! { + pub static FAIL_NEXT_SUCCESSFUL_EXECUTION: Cell = const { Cell::new(false) }; + } +} + impl Platform where C: CoreRPCLike, @@ -108,6 +123,29 @@ where ); } + // PROPOSER-SIDE ONLY (consensus-invisible, hence no protocol-version + // gate): when building a proposal, mark the state before this + // transition. Execution can write into the shared block transaction + // before failing (the address-input fee flow is apply-then-check), and + // a transition whose result strips it from the block + // (`TxAction::Removed`) must leave no trace in the state the proposal's + // app hash is computed over — otherwise the gossiped block omits the + // transition while the advertised app hash includes its writes, no + // validator can reproduce the hash, and every proposer carrying the + // transition burns its round (mainnet evo1 stalls of 2026-08-14/15, + // after heights 415652 and 415661). + // + // The validation path (`proposing_state_transitions == false`) is + // deliberately untouched: rolling back there would change what state a + // received block evaluates to, which is a consensus change that must + // ride a protocol-version gate (it does, from v14). This proposer-side + // rollback only changes which blocks this node BUILDS — the published + // block and app hash are exactly what any un-upgraded validator + // computes from that block, so mixed networks cannot diverge. + if proposing_state_transitions { + transaction.set_savepoint(); + } + // Validate state transition and produce an execution event let execution_result = process_state_transition( &platform_ref, @@ -137,6 +175,47 @@ where }) .unwrap_or_else(error_to_internal_error_execution_result); + #[cfg(test)] + let execution_result = if matches!( + execution_result, + StateTransitionExecutionResult::SuccessfulExecution { .. } + ) + && test_fault_injection::FAIL_NEXT_SUCCESSFUL_EXECUTION + .with(|flag| flag.replace(false)) + { + StateTransitionExecutionResult::InternalError( + "injected post-apply failure (test_fault_injection)".to_string(), + ) + } else { + execution_result + }; + + if proposing_state_transitions { + match &execution_result { + StateTransitionExecutionResult::InternalError(_) + | StateTransitionExecutionResult::UnpaidConsensusError(_) => { + // This transition will be stripped from the proposal + // (`TxAction::Removed`), so none of its writes may remain + // in the state the app hash is computed over. A rollback + // failure means the proposal can no longer match the + // block — fail it rather than continue on leaked state. + transaction.rollback_to_savepoint().map_err(|e| { + drive::grovedb::error::Error::StorageError(RocksDBError(e)) + })?; + } + _ => { + // The transition stays in the block, so its writes stay. + // Its savepoint is intentionally left on the stack: + // RocksDB exposes no pop-without-rollback, leftover + // savepoints are inert for commit, and the per-round + // proposal transaction they live in is dropped when the + // round ends. The genesis re-proposal path — the one other + // consumer of this stack — drains the whole stack rather + // than popping once, so this residue cannot redirect it. + } + } + } + // Store metrics let elapsed_time = start_time.elapsed() + decoding_elapsed_time; diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shield/tests.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shield/tests.rs index 168c813aa9a..cb3b54b13d8 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shield/tests.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shield/tests.rs @@ -1747,4 +1747,263 @@ mod tests { ); } } + + /// MAINNET HALT HOTFIX (evo1, 2026-08-14/15: ~2h stalls after 415652 and 415661). + /// + /// Execution can write into the shared block transaction before failing (the address-input + /// fee flow is apply-then-check), and a transition whose result strips it from the block + /// (`InternalError` -> `TxAction::Removed`) left those writes behind: the proposer gossiped + /// a block WITHOUT the transition while advertising an app hash computed WITH its writes, + /// so no validator could reproduce the hash and every proposer carrying the transition + /// burned its round. + /// + /// The hotfix rolls such transitions back on the PROPOSING path only. These tests pin both + /// halves of that contract: the proposing path leaves no trace, and the validating path is + /// byte-identical to v4.1.0 (rolling back there would change what a received block + /// evaluates to — a consensus change that must ride a protocol-version gate, not a hotfix). + /// + /// Both tests use a fault hook rather than a real under-funded shield so they are + /// independent of the fee-estimation constants that made the mainnet transitions fail + /// (dashpay/grovedb#812). + mod proposer_rollback_hotfix { + use super::*; + use crate::execution::platform_events::state_transition_processing::test_fault_injection::FAIL_NEXT_SUCCESSFUL_EXECUTION; + use crate::execution::validation::state_transition::state_transitions::test_helpers::insert_dummy_encrypted_notes; + use dpp::block::block_info::BlockInfo; + + /// Note count on mainnet's shielded commitment tree around the halt. + const MAINNET_NOTES: u64 = 494; + + struct Bundle { + actions: Vec, + shield_amount: u64, + anchor: [u8; 32], + proof: Vec, + binding_sig: [u8; 64], + } + + fn build_bundle() -> Bundle { + let mut rng = OsRng; + let pk = get_proving_key(); + let sk = SpendingKey::from_bytes([0u8; 32]).unwrap(); + let fvk = FullViewingKey::from(&sk); + let recipient = fvk.address_at(0u32, Scope::External); + + let mut builder = Builder::::new( + BundleType::Transactional { + flags: OrchardFlags::SPENDS_DISABLED, + bundle_required: false, + }, + Anchor::empty_tree(), + ); + builder + .add_output(None, recipient, NoteValue::from_raw(5000u64), [0u8; 36]) + .unwrap(); + let (unauthorized, _) = builder.build::(&mut rng).unwrap().unwrap(); + let commitment: [u8; 32] = unauthorized.commitment().into(); + let sighash = compute_platform_sighash(&commitment, &[]); + let proven = unauthorized.create_proof(pk, &mut rng).unwrap(); + let bundle = proven.apply_signatures(rng, sighash, &[]).unwrap(); + + let (actions, _flags, value_balance, anchor, proof, binding_sig) = + serialize_authorized_bundle_with_flags(&bundle); + assert!( + value_balance < 0, + "a shield must have negative value balance" + ); + Bundle { + actions, + shield_amount: (-value_balance) as u64, + anchor, + proof, + binding_sig, + } + } + + async fn build_signed( + b: &Bundle, + signer: &TestAddressSigner, + addr: PlatformAddress, + declared_input: u64, + ) -> StateTransition { + let mut inputs = BTreeMap::new(); + inputs.insert(addr, (1 as AddressNonce, declared_input)); + + let mut st = StateTransition::Shield(ShieldTransition::V0(ShieldTransitionV0 { + inputs: inputs.clone(), + actions: b.actions.clone(), + amount: b.shield_amount, + anchor: b.anchor, + proof: b.proof.clone(), + binding_signature: b.binding_sig, + fee_strategy: AddressFundsFeeStrategy::from(vec![ + AddressFundsFeeStrategyStep::DeductFromInput(0), + ]), + user_fee_increase: 0, + input_witnesses: vec![], + })); + let signable = st.signable_bytes().expect("should compute signable bytes"); + let mut witnesses: Vec = Vec::with_capacity(inputs.len()); + for a in inputs.keys() { + witnesses.push( + signer + .sign_create_witness(a, &signable) + .await + .expect("sign"), + ); + } + if let StateTransition::Shield(ShieldTransition::V0(ref mut v0)) = st { + v0.input_witnesses = witnesses; + } + st + } + + struct RunOutcome { + dropped_as_internal_error: bool, + pool_delta: i128, + notes_delta: i128, + hash_changed: bool, + } + + /// Run a fully-funded shield with the post-apply fault injected, on the proposing or + /// validating path, and report what it left behind. + async fn run_injected(proposing: bool) -> RunOutcome { + let pv = PlatformVersion::latest(); + let b = build_bundle(); + // Fully funded: without the injected failure this shield would execute and land. + let headroom = 5_000_000_000u64; + + let mut platform = setup_platform(); + insert_dummy_encrypted_notes(&platform, MAINNET_NOTES); + let mut signer = TestAddressSigner::new(); + let addr = signer.add_p2pkh([1u8; 32]); + let declared_input = b.shield_amount + headroom; + setup_address_with_balance_and_system_credits(&mut platform, addr, 0, declared_input); + + let st = build_signed(&b, &signer, addr, declared_input).await; + let bytes = st.serialize_to_bytes().expect("serialize"); + let state = platform.state.load(); + let transaction = platform.drive.grove.start_transaction(); + + let pool_before = platform + .drive + .read_shielded_pool_total_balance(Some(&transaction), &mut vec![], pv) + .expect("pool balance"); + let notes_before = platform + .drive + .shielded_pool_notes_count(Some(&transaction), &mut vec![], pv) + .expect("notes count"); + let hash_before = platform + .drive + .grove + .root_hash(Some(&transaction), &pv.drive.grove_version) + .unwrap() + .expect("root hash"); + + FAIL_NEXT_SUCCESSFUL_EXECUTION.with(|flag| flag.set(true)); + + let result = platform + .platform + .process_raw_state_transitions( + &vec![bytes], + &state, + &BlockInfo::default(), + &transaction, + pv, + proposing, + None, + ) + .expect("processing must not be a block-level error"); + + assert!( + !FAIL_NEXT_SUCCESSFUL_EXECUTION.with(|flag| flag.get()), + "sanity: the injection must have been consumed (the shield must have executed \ + successfully before being overridden)" + ); + + let dropped_as_internal_error = matches!( + result.execution_results().first(), + Some(StateTransitionExecutionResult::InternalError(_)) + ); + + let pool_after = platform + .drive + .read_shielded_pool_total_balance(Some(&transaction), &mut vec![], pv) + .expect("pool balance"); + let notes_after = platform + .drive + .shielded_pool_notes_count(Some(&transaction), &mut vec![], pv) + .expect("notes count"); + let hash_after = platform + .drive + .grove + .root_hash(Some(&transaction), &pv.drive.grove_version) + .unwrap() + .expect("root hash"); + + RunOutcome { + dropped_as_internal_error, + pool_delta: pool_after as i128 - pool_before as i128, + notes_delta: notes_after as i128 - notes_before as i128, + hash_changed: hash_after != hash_before, + } + } + + /// The fix: a transition dropped as `InternalError` while PROPOSING must leave the + /// shielded pool, the note commitment tree, and the root hash untouched — the proposal + /// then omits the transition AND its app hash omits its writes, so any validator + /// (including un-upgraded v4.1.0 ones) reproduces the hash and the round commits. + #[tokio::test] + async fn proposing_must_not_leave_state_of_dropped_transition() { + let outcome = run_injected(true).await; + assert!( + outcome.dropped_as_internal_error, + "expected the injected InternalError" + ); + assert_eq!( + outcome.pool_delta, 0, + "STATE LEAK: a transition dropped from the proposal still credited the \ + shielded pool" + ); + assert_eq!( + outcome.notes_delta, 0, + "STATE LEAK: a transition dropped from the proposal still appended note \ + commitments" + ); + assert!( + !outcome.hash_changed, + "APP HASH POISONED: a transition dropped from the proposal changed the app \ + hash; validators replaying the block (which omits it) can never reproduce \ + this hash and the chain stalls" + ); + } + + /// The consensus-invisibility guarantee: the VALIDATING path must behave exactly as + /// v4.1.0 did — no savepoint, no rollback, the leak preserved. Rolling back here would + /// change what state a received block evaluates to, i.e. a consensus change: an + /// upgraded validator would then disagree with un-upgraded ones about any block that + /// carries such a transition. That change is version-gated to protocol v14 and MUST + /// NOT be active in this hotfix. If this test ever fails because the deltas became + /// zero, the hotfix has silently become a fork. + #[tokio::test] + async fn validating_must_behave_exactly_as_v4_1_0() { + let outcome = run_injected(false).await; + assert!( + outcome.dropped_as_internal_error, + "expected the injected InternalError" + ); + assert_eq!( + outcome.pool_delta, 5000, + "the validating path must keep v4.1.0 behavior bit-for-bit (leak preserved)" + ); + assert_eq!( + outcome.notes_delta, 2, + "the validating path must keep v4.1.0 behavior bit-for-bit (leak preserved)" + ); + assert!( + outcome.hash_changed, + "the validating path must keep v4.1.0 behavior bit-for-bit (leak preserved)" + ); + } + } } diff --git a/packages/rs-drive-abci/src/mimic/mod.rs b/packages/rs-drive-abci/src/mimic/mod.rs index cf37b659df3..e00e04c5d6e 100644 --- a/packages/rs-drive-abci/src/mimic/mod.rs +++ b/packages/rs-drive-abci/src/mimic/mod.rs @@ -351,6 +351,11 @@ impl FullAbciApplication<'_, C> { transaction .rollback_to_savepoint() .expect("expected to rollback to savepoint"); + // Drain per-transition savepoints left by the proposer-side rollback in + // process_raw_state_transitions_v0 so we land on the post-init-chain state, + // matching the genesis-path drain in prepare_proposal/process_proposal. The + // root-hash assertion below verifies the landing point. + while transaction.rollback_to_savepoint().is_ok() {} transaction.set_savepoint(); let start_root_hash = self From 7f490b2d268010d1c98404849f13661c484257ec Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Mon, 17 Aug 2026 18:39:47 +0700 Subject: [PATCH 2/4] test(drive-abci): pin the real under-funded-shield rollback on the proposing path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fund a shield at the edge of the estimated-vs-actual fee band (no fault hook) and assert the proposing path leaves state consistent with the outcome. On this line it reproduces the exact mainnet halt case: the transition passes estimated-fee validation, fails the actual-fee coverage guard at execution, is dropped as InternalError — and the root hash is unchanged. The match on the execution result keeps the test valid if fee constants shift: a validation reject must also leave no trace, and only a genuine success may change state. Co-Authored-By: Claude Fable 5 --- .../state_transitions/shield/tests.rs | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shield/tests.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shield/tests.rs index cb3b54b13d8..4d4c9f7d938 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shield/tests.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shield/tests.rs @@ -2005,5 +2005,95 @@ mod tests { "the validating path must keep v4.1.0 behavior bit-for-bit (leak preserved)" ); } + + /// The real mainnet scenario, no fault hook: a shield funded at the edge of the + /// estimated-vs-actual fee band measured on v4.2-dev (actual metered fee + /// 177,215,760 credits at 494 notes; headroom one credit below). The grovedb pin + /// differs on the v4.1 line so the exact constants may shift; whatever this funding + /// level produces here, the proposing path must leave state consistent with it: + /// a dropped or rejected transition leaves NO trace (this was the halt), and only a + /// genuinely successful one changes state. + #[tokio::test] + async fn proposing_real_underfunded_shield_leaves_no_trace() { + let pv = PlatformVersion::latest(); + let b = build_bundle(); + let headroom = 177_215_759u64; + + let mut platform = setup_platform(); + insert_dummy_encrypted_notes(&platform, MAINNET_NOTES); + let mut signer = TestAddressSigner::new(); + let addr = signer.add_p2pkh([1u8; 32]); + let declared_input = b.shield_amount + headroom; + setup_address_with_balance_and_system_credits(&mut platform, addr, 0, declared_input); + + let st = build_signed(&b, &signer, addr, declared_input).await; + let bytes = st.serialize_to_bytes().expect("serialize"); + let state = platform.state.load(); + let transaction = platform.drive.grove.start_transaction(); + + let hash_before = platform + .drive + .grove + .root_hash(Some(&transaction), &pv.drive.grove_version) + .unwrap() + .expect("root hash"); + + let result = platform + .platform + .process_raw_state_transitions( + &vec![bytes], + &state, + &BlockInfo::default(), + &transaction, + pv, + true, // proposing, exactly as prepare_proposal does + None, + ) + .expect("processing must not be a block-level error"); + + let hash_after = platform + .drive + .grove + .root_hash(Some(&transaction), &pv.drive.grove_version) + .unwrap() + .expect("root hash"); + + println!( + "outcome at band-edge headroom: {:?}", + result.execution_results().first() + ); + match result.execution_results().first() { + Some(StateTransitionExecutionResult::InternalError(msg)) => { + // The mainnet halt case: accepted by estimated-fee validation, failed by + // actual-fee execution, dropped from the proposal. Must leave no trace. + assert!( + msg.contains("not fully covered"), + "expected the fee coverage guard, got: {msg}" + ); + assert_eq!( + hash_before, hash_after, + "APP HASH POISONED: the exact mainnet halt scenario leaked state on \ + the proposing path" + ); + } + Some(StateTransitionExecutionResult::UnpaidConsensusError(_)) => { + // Fee constants on this line put the estimate above this funding level: + // rejected before execution. Fine — but still must leave no trace. + assert_eq!( + hash_before, hash_after, + "a validation-rejected shield must not touch state" + ); + } + Some(StateTransitionExecutionResult::SuccessfulExecution { .. }) => { + // Fee constants on this line put the actual fee at or below this funding + // level: the shield legitimately landed, so state MUST have changed. + assert_ne!( + hash_before, hash_after, + "a successful shield must change state" + ); + } + other => panic!("unexpected execution result: {other:?}"), + } + } } } From c95e8269375b1216eecf39f877067e7da93f90a7 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Mon, 17 Aug 2026 18:50:29 +0700 Subject: [PATCH 3/4] refactor(drive-abci): scope proposer-side savepoints to non-genesis heights MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the genesis savepoint-stack drain with not creating per-ST savepoints at the genesis height in the first place. The genesis re-proposal path keeps its original single-savepoint discipline (init_chain sets one savepoint, each round rewinds to it with one rollback) with prepare_proposal, process_proposal and mimic reverted to their pre-hotfix state — no drain loops, no swallowed errors, no compensating logic at a distance. At every other height each proposal round runs in a freshly started transaction, so savepoints left by kept transitions are provably inert: they die with a dropped round or ride through commit as markers. Trade-off, accepted: no halt protection for a state transition inside a genesis-height block itself. Irrelevant to any running network; a new devnet that trips it restarts. Co-Authored-By: Claude Fable 5 --- .../src/abci/handler/prepare_proposal.rs | 7 --- .../src/abci/handler/process_proposal.rs | 7 --- .../process_raw_state_transitions/v0/mod.rs | 63 ++++++++++--------- packages/rs-drive-abci/src/mimic/mod.rs | 5 -- 4 files changed, 35 insertions(+), 47 deletions(-) diff --git a/packages/rs-drive-abci/src/abci/handler/prepare_proposal.rs b/packages/rs-drive-abci/src/abci/handler/prepare_proposal.rs index 500f9484e30..10a4bd95631 100644 --- a/packages/rs-drive-abci/src/abci/handler/prepare_proposal.rs +++ b/packages/rs-drive-abci/src/abci/handler/prepare_proposal.rs @@ -143,13 +143,6 @@ where if let Some(tx) = transaction_guard.as_ref() { tx.rollback_to_savepoint() .map_err(|e| drive::grovedb::error::Error::StorageError(RocksDBError(e)))?; - // Drain the rest of the savepoint stack: the state-transition loop leaves one - // savepoint per transition executed while proposing (see the proposer-side - // rollback in process_raw_state_transitions_v0), so a single rollback may only - // rewind to the last transition of the previous round. Every savepoint on this - // stack records the post-init-chain state or later, and the bottom one records - // exactly it, so draining until empty always lands on the post-init-chain state. - while tx.rollback_to_savepoint().is_ok() {} tx.set_savepoint(); } transaction_guard diff --git a/packages/rs-drive-abci/src/abci/handler/process_proposal.rs b/packages/rs-drive-abci/src/abci/handler/process_proposal.rs index cb3c41cc79a..a64aba5013b 100644 --- a/packages/rs-drive-abci/src/abci/handler/process_proposal.rs +++ b/packages/rs-drive-abci/src/abci/handler/process_proposal.rs @@ -174,13 +174,6 @@ where if let Some(tx) = transaction_guard.as_ref() { tx.rollback_to_savepoint() .map_err(|e| drive::grovedb::error::Error::StorageError(RocksDBError(e)))?; - // Drain the rest of the savepoint stack: the state-transition loop leaves one - // savepoint per transition executed while proposing (see the proposer-side - // rollback in process_raw_state_transitions_v0), so a single rollback may only - // rewind to the last transition of the previous round. Every savepoint on this - // stack records the post-init-chain state or later, and the bottom one records - // exactly it, so draining until empty always lands on the post-init-chain state. - while tx.rollback_to_savepoint().is_ok() {} tx.set_savepoint(); } transaction_guard diff --git a/packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/process_raw_state_transitions/v0/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/process_raw_state_transitions/v0/mod.rs index 3990ae631bb..f32916a053c 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/process_raw_state_transitions/v0/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/process_raw_state_transitions/v0/mod.rs @@ -84,6 +84,33 @@ where let state_transition_container = self.decode_raw_state_transitions(raw_state_transitions, platform_version)?; + // PROPOSER-SIDE ONLY (consensus-invisible, hence no protocol-version gate): while + // building a proposal, wrap each executed state transition in a savepoint and roll + // back if its result strips it from the block (`TxAction::Removed`). Execution can + // write into the shared block transaction before failing (the address-input fee flow + // is apply-then-check), and without the rollback the gossiped block omits the + // transition while the advertised app hash includes its writes — no validator can + // reproduce the hash, and every proposer carrying the transition burns its round + // (mainnet evo1 stalls of 2026-08-14/15, after heights 415652 and 415661). + // + // The validation path (`proposing_state_transitions == false`) is deliberately + // untouched: rolling back there would change what state a received block evaluates + // to, which is a consensus change that must ride a protocol-version gate (it does, + // from v14). This proposer-side rollback only changes which blocks this node BUILDS — + // the published block and app hash are exactly what any un-upgraded validator + // computes from that block, so mixed networks cannot diverge. + // + // The genesis height is excluded because its re-proposal path relies on a + // single-savepoint discipline: init_chain sets one savepoint, and each genesis round + // rewinds to it with one `rollback_to_savepoint()` (see prepare_proposal / + // process_proposal). Savepoints of KEPT transitions stay on the stack — RocksDB + // exposes no pop-without-rollback — and extra savepoints on the genesis transaction + // would redirect that rewind. At every other height each proposal round runs in a + // freshly started transaction that is either committed (leftover savepoints are inert + // markers) or dropped when the round ends, so the residue can affect nothing. + let rollback_dropped_transitions = + proposing_state_transitions && block_info.height != self.config.abci.genesis_height; + let mut processing_result = StateTransitionsProcessingResult::default(); for decoded_state_transition in state_transition_container.into_iter() { @@ -123,26 +150,9 @@ where ); } - // PROPOSER-SIDE ONLY (consensus-invisible, hence no protocol-version - // gate): when building a proposal, mark the state before this - // transition. Execution can write into the shared block transaction - // before failing (the address-input fee flow is apply-then-check), and - // a transition whose result strips it from the block - // (`TxAction::Removed`) must leave no trace in the state the proposal's - // app hash is computed over — otherwise the gossiped block omits the - // transition while the advertised app hash includes its writes, no - // validator can reproduce the hash, and every proposer carrying the - // transition burns its round (mainnet evo1 stalls of 2026-08-14/15, - // after heights 415652 and 415661). - // - // The validation path (`proposing_state_transitions == false`) is - // deliberately untouched: rolling back there would change what state a - // received block evaluates to, which is a consensus change that must - // ride a protocol-version gate (it does, from v14). This proposer-side - // rollback only changes which blocks this node BUILDS — the published - // block and app hash are exactly what any un-upgraded validator - // computes from that block, so mixed networks cannot diverge. - if proposing_state_transitions { + // Mark the state we can return to if this transition's result strips + // it from the block (see `rollback_dropped_transitions` above). + if rollback_dropped_transitions { transaction.set_savepoint(); } @@ -190,7 +200,7 @@ where execution_result }; - if proposing_state_transitions { + if rollback_dropped_transitions { match &execution_result { StateTransitionExecutionResult::InternalError(_) | StateTransitionExecutionResult::UnpaidConsensusError(_) => { @@ -205,13 +215,10 @@ where } _ => { // The transition stays in the block, so its writes stay. - // Its savepoint is intentionally left on the stack: - // RocksDB exposes no pop-without-rollback, leftover - // savepoints are inert for commit, and the per-round - // proposal transaction they live in is dropped when the - // round ends. The genesis re-proposal path — the one other - // consumer of this stack — drains the whole stack rather - // than popping once, so this residue cannot redirect it. + // Its savepoint is intentionally left on the stack (see + // `rollback_dropped_transitions` above: no + // pop-without-rollback exists, and at non-genesis heights + // the residue is inert). } } } diff --git a/packages/rs-drive-abci/src/mimic/mod.rs b/packages/rs-drive-abci/src/mimic/mod.rs index e00e04c5d6e..cf37b659df3 100644 --- a/packages/rs-drive-abci/src/mimic/mod.rs +++ b/packages/rs-drive-abci/src/mimic/mod.rs @@ -351,11 +351,6 @@ impl FullAbciApplication<'_, C> { transaction .rollback_to_savepoint() .expect("expected to rollback to savepoint"); - // Drain per-transition savepoints left by the proposer-side rollback in - // process_raw_state_transitions_v0 so we land on the post-init-chain state, - // matching the genesis-path drain in prepare_proposal/process_proposal. The - // root-hash assertion below verifies the landing point. - while transaction.rollback_to_savepoint().is_ok() {} transaction.set_savepoint(); let start_root_hash = self From 3978bdf9559dfa3a1a421236f1b1724a6a10235d Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Mon, 17 Aug 2026 19:47:15 +0700 Subject: [PATCH 4/4] refactor(drive-abci): make the rollback classification exhaustive Enumerate every StateTransitionExecutionResult variant in the proposer-side rollback match instead of a wildcard, so adding a new execution result forces an explicit savepoint decision at the point where the rollback classification must mirror prepare_proposal's TxAction classification. Suggested by review on #4409. Co-Authored-By: Claude Fable 5 --- .../process_raw_state_transitions/v0/mod.rs | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/process_raw_state_transitions/v0/mod.rs b/packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/process_raw_state_transitions/v0/mod.rs index f32916a053c..d04d486dd96 100644 --- a/packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/process_raw_state_transitions/v0/mod.rs +++ b/packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/process_raw_state_transitions/v0/mod.rs @@ -213,13 +213,26 @@ where drive::grovedb::error::Error::StorageError(RocksDBError(e)) })?; } - _ => { - // The transition stays in the block, so its writes stay. - // Its savepoint is intentionally left on the stack (see + StateTransitionExecutionResult::SuccessfulExecution { .. } + | StateTransitionExecutionResult::PaidConsensusError { .. } => { + // The transition stays in the block + // (`TxAction::Unmodified`), so its writes stay. Its + // savepoint is intentionally left on the stack (see // `rollback_dropped_transitions` above: no // pop-without-rollback exists, and at non-genesis heights // the residue is inert). } + StateTransitionExecutionResult::NotExecuted(_) => { + // Delayed to a later block (`TxAction::Delayed`) without + // having been executed: nothing was written since the + // savepoint, so rolling back and leaving it are + // equivalent. Leave it, like the kept outcomes above. + // + // Deliberately exhaustive: a new execution result variant + // must make an explicit savepoint decision here — the + // rollback classification must match the `TxAction` + // classification in `prepare_proposal`. + } } }