From 65599dfcc5cd8b1c622a0e862ea4782f387ef5bc Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Thu, 11 Jun 2026 22:50:05 +0200 Subject: [PATCH] fix(drive): verify identity-create-from-shielded-pool results without unbounded terminal-key queries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The type-20 verifier arm rebuilt the merged {nullifiers, full-identity} query and verified it with verify_query_with_absence_proof, which enumerates the query's terminal keys — impossible for full_identity_query's unbounded all-keys RangeFull. grovedb rejected the query outright ("terminal keys are not supported with unbounded ranges"), so every honest type-20 result proof failed client-side even though the transition executed on-chain, leaving an orphaned-but-live identity the client never persisted. Verify with strict GroveDb::verify_query instead (succinctness still rejects proofs padded with extra branches); derive nullifier spend statuses from membership in the proved result set and reject unrequested nullifier entries. The prove side is unchanged. Adds a drive-abci regression test that executes a type-20 success action through the real execute_event path (synthetic action, no Halo 2 proving), commits, then round-trips prove_state_transition → verify_state_transition_was_executed_with_proof — failing with the exact devnet error before the fix. Co-Authored-By: Claude Fable 5 --- .../tests.rs | 163 +++++++++++++++++- .../prove/prove_state_transition/v0/mod.rs | 6 +- .../v0/mod.rs | 64 ++++--- 3 files changed, 205 insertions(+), 28 deletions(-) diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_create_from_shielded_pool/tests.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_create_from_shielded_pool/tests.rs index 3dc9319e80..02e7968afb 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_create_from_shielded_pool/tests.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_create_from_shielded_pool/tests.rs @@ -11,11 +11,15 @@ //! path (the fallback Unshield, pool->address minus penalty): the converter ops applied through a //! real Drive keep `calculate_total_credits_balance().ok()` balanced (the end-of-block invariant //! that halts the chain) — the regression guard for the `AddToSystemCredits` over-mint. +//! - The execute -> prove -> verify result-proof roundtrip for the success path (synthetic action, +//! no Halo 2 proving): the regression guard for the devnet bug where the verifier's absence-proof +//! verification choked on the merged {nullifiers, full-identity} query ("terminal keys are not +//! supported with unbounded ranges"), so every honest type-20 result proof failed client-side. //! -//! The full build->prove->execute->prove/verify happy path (real Orchard proof + the strict merged -//! nullifier+identity proof roundtrip) is deferred to the shared shielded-strategy harness, a -//! pre-existing repo-wide TODO that is disabled for every shielded transition (the shielded -//! `OperationType` build handlers are commented out in `strategy.rs`). +//! The full build->prove->execute->prove/verify happy path (real Orchard proof) is deferred to the +//! shared shielded-strategy harness, a pre-existing repo-wide TODO that is disabled for every +//! shielded transition (the shielded `OperationType` build handlers are commented out in +//! `strategy.rs`). use super::state::v0::IdentityCreateFromShieldedPoolStateTransitionStateValidationV0; use super::transform_into_action::v0::IdentityCreateFromShieldedPoolStateTransitionTransformIntoActionValidationV0; @@ -850,3 +854,154 @@ fn failure_path_unshield_converter_ops_preserve_sum_tree_credit_conservation() { "credit supply must be conserved after the fallback pool->address unshield; got {balance}" ); } + +/// Regression for the 2026-06-11 devnet (paloma) type-20 proof-fetch failure: an EXECUTED +/// `IdentityCreateFromShieldedPool` must round-trip `prove_state_transition` -> +/// `verify_state_transition_was_executed_with_proof`. The verifier used +/// `verify_query_with_absence_proof` over the merged {nullifiers, full-identity} query, but +/// `full_identity_query`'s all-keys sub-query is an unbounded RangeFull and absence verification +/// enumerates the query's terminal keys — so EVERY honest type-20 result proof was rejected +/// client-side with "terminal keys are not supported with unbounded ranges", while the transition +/// itself executed on-chain (leaving an orphaned-but-live identity the client never persisted). +/// +/// Executes the success action through the real `execute_event` path (synthetic action — no Halo 2 +/// proving needed), commits, then generates the result proof and verifies it. +#[test] +fn executed_transition_result_proof_roundtrips() { + use crate::execution::types::execution_event::ExecutionEvent; + use crate::platform_types::event_execution_result::EventExecutionResult; + use dpp::block::epoch::Epoch; + use dpp::fee::default_costs::CachedEpochIndexFeeVersions; + use dpp::state_transition::identity_create_from_shielded_pool_transition::accessors::IdentityCreateFromShieldedPoolTransitionAccessorsV0; + use dpp::state_transition::proof_result::StateTransitionProofResult; + use dpp::state_transition::StateTransition; + use drive::drive::Drive; + + let platform_version = PlatformVersion::latest(); + let platform = setup_platform(); + let block_info = BlockInfo::default(); + + set_pool_total_balance(&platform, DENOMINATION * 10); + insert_anchor_into_state(&platform, &ANCHOR); + let min_notes = platform_version + .drive_abci + .validation_and_processing + .event_constants + .minimum_pool_notes_for_outgoing; + insert_dummy_encrypted_notes(&platform, min_notes.max(1)); + + // Applying `AddNewIdentity` parses the key bytes, so the key must be a real secp256k1 point. + let (valid_master_key, _) = + IdentityPublicKey::random_ecdsa_master_authentication_key(0, Some(21), platform_version) + .expect("expected a valid master key"); + let key_in_creation = IdentityPublicKeyInCreation::V0(IdentityPublicKeyInCreationV0 { + id: 0, + key_type: valid_master_key.key_type(), + purpose: valid_master_key.purpose(), + security_level: valid_master_key.security_level(), + contract_bounds: None, + read_only: false, + data: valid_master_key.data().clone(), + signature: BinaryData::default(), + }); + + let st = transition(vec![key_in_creation], vec![action(60), action(61)]); + let expected_identity_id = st.identity_id(); + let expected_nullifiers: Vec> = st.nullifiers(); + let mut execution_context = + StateTransitionExecutionContext::default_for_platform_version(platform_version) + .expect("execution context"); + let success_action = + build_success_action(&platform, &st, &mut execution_context, platform_version); + + let event = ExecutionEvent::create_from_state_transition_action( + StateTransitionAction::IdentityCreateFromShieldedPoolAction(success_action), + None, + &Epoch::new(0).unwrap(), + execution_context, + platform_version, + ) + .expect("create execution event"); + + let transaction = platform.drive.grove.start_transaction(); + let fee_versions = CachedEpochIndexFeeVersions::new(); + let exec_result = platform + .platform + .execute_event( + event, + vec![], + &block_info, + &transaction, + None, + platform_version, + &fee_versions, + ) + .expect("execute_event should not error"); + assert_matches!( + exec_result, + EventExecutionResult::SuccessfulPaidExecution(..), + "the success action must execute" + ); + + // Commit so prove_state_transition reads the post-execution committed state. + platform + .drive + .grove + .commit_transaction(transaction) + .unwrap() + .expect("expected to commit transaction"); + + let transition = StateTransition::IdentityCreateFromShieldedPool(st); + + let proof_result = platform + .drive + .prove_state_transition(&transition, None, platform_version) + .expect("expected to generate the result proof"); + let proof_bytes = proof_result + .into_data() + .expect("expected proof data, not an error"); + + let (root_hash, result) = Drive::verify_state_transition_was_executed_with_proof( + &transition, + &block_info, + &proof_bytes, + &|_| Ok(None), + platform_version, + ) + .expect( + "the honest result proof must verify — absence-proof verification rejects the unbounded \ + all-keys identity sub-query", + ); + assert_ne!(root_hash, [0u8; 32], "root hash should not be zeroed"); + + let StateTransitionProofResult::VerifiedIdentityWithShieldedNullifiers(identity, statuses) = + result + else { + panic!("expected VerifiedIdentityWithShieldedNullifiers, got {result:?}"); + }; + + assert_eq!( + identity.id(), + expected_identity_id, + "the proven identity must be the one derived from the published nullifiers" + ); + assert_eq!( + identity.public_keys().len(), + 1, + "the proven identity must hold exactly the transition's declared key" + ); + + assert_eq!( + statuses.len(), + expected_nullifiers.len(), + "should have one status per nullifier" + ); + for (nf, is_spent) in &statuses { + assert!(is_spent, "nullifier {} should be spent", hex::encode(nf)); + assert!( + expected_nullifiers.contains(nf), + "proved nullifier {} should be one of the transition's nullifiers", + hex::encode(nf) + ); + } +} diff --git a/packages/rs-drive/src/prove/prove_state_transition/v0/mod.rs b/packages/rs-drive/src/prove/prove_state_transition/v0/mod.rs index eae1870474..0086c6b061 100644 --- a/packages/rs-drive/src/prove/prove_state_transition/v0/mod.rs +++ b/packages/rs-drive/src/prove/prove_state_transition/v0/mod.rs @@ -439,8 +439,10 @@ impl Drive { // Prove BOTH the spent nullifiers AND the newly-created identity in a single merged // multi-root proof. Built STRICT from day one (per #3812): the verifier rebuilds this - // exact merged query and verifies it with `verify_query_with_absence_proof`, so the - // proof cannot carry any branch beyond {nullifiers, identity}. + // exact merged query and verifies it with `verify_query` (succinctness on), so the + // proof cannot carry any branch beyond {nullifiers, identity}. The absence-proof + // variant is unusable here: it enumerates the query's terminal keys, which is + // impossible for `full_identity_query`'s unbounded all-keys range. let nullifier_keys: Vec> = st.nullifiers(); let mut nf_query = grovedb::Query::new(); nf_query.insert_keys(nullifier_keys); diff --git a/packages/rs-drive/src/verify/state_transition/verify_state_transition_was_executed_with_proof/v0/mod.rs b/packages/rs-drive/src/verify/state_transition/verify_state_transition_was_executed_with_proof/v0/mod.rs index fd11590abc..e09dda4c9f 100644 --- a/packages/rs-drive/src/verify/state_transition/verify_state_transition_was_executed_with_proof/v0/mod.rs +++ b/packages/rs-drive/src/verify/state_transition/verify_state_transition_was_executed_with_proof/v0/mod.rs @@ -1605,7 +1605,7 @@ impl Drive { use dpp::state_transition::identity_create_from_shielded_pool_transition::accessors::IdentityCreateFromShieldedPoolTransitionAccessorsV0; use dpp::state_transition::identity_create_from_shielded_pool_transition::derive_identity_id_from_actions; use dpp::state_transition::proof_result::StateTransitionProofResult::VerifiedIdentityWithShieldedNullifiers; - use std::collections::BTreeMap; + use std::collections::{BTreeMap, BTreeSet}; // Recompute the id from the actions (the canonical value) instead of trusting the // wire field, and reject a tampered transition whose wire id doesn't match — so a @@ -1638,25 +1638,28 @@ impl Drive { )?; identity_pq.query.limit = None; - let mut merged_pq = grovedb::PathQuery::merge( + let merged_pq = grovedb::PathQuery::merge( vec![&nullifier_pq, &identity_pq], &platform_version.drive.grove_version, )?; - // STRICT verification: `verify_query_with_absence_proof` requires a limit, but - // `merge` leaves it None. Use an unreachable `u16::MAX` so the per-layer succinctness - // check (which rejects extra proof branches — the whole point of building this strict - // from day one, cf. #3812) runs fully on every layer; a smaller limit could break the - // result loop early and falsely reject honest proofs. The limit does NOT relax - // extra-data rejection. - merged_pq.query.limit = Some(u16::MAX); - - let (root_hash, proved_key_values) = - grovedb::GroveDb::verify_query_with_absence_proof( - proof, - &merged_pq, - &platform_version.drive.grove_version, - )?; + // STRICT verification via `verify_query` (succinctness on). Unlike the other + // shielded merged queries (which target only explicit keys and go through + // `verify_merged_query_strict`), this one embeds `full_identity_query`, whose + // all-keys sub-query is an unbounded RangeFull — and + // `verify_query_with_absence_proof` enumerates the query's terminal keys, which + // is impossible for unbounded ranges ("terminal keys are not supported with + // unbounded ranges"). Absence synthesis isn't needed here anyway: every queried + // element (the spent nullifiers and the created identity) must be PRESENT, so + // presence is checked directly against the result set below. The succinctness + // check still rejects proofs padded with branches beyond {nullifiers, identity} + // (the strict-from-day-one guarantee of #3812), and the limit stays None exactly + // as the prove side built it, so no layer's result loop can break early. + let (root_hash, proved_key_values) = grovedb::GroveDb::verify_query( + proof, + &merged_pq, + &platform_version.drive.grove_version, + )?; // Partition the proved key/values by PATH (NOT key length — nullifier keys and the // identity id are both 32 bytes): nullifier-tree entries vs the identity subtrees @@ -1667,14 +1670,23 @@ impl Drive { let identity_path = identity_path(identity_id.as_slice()); let identity_keys_path = identity_key_tree_path(identity_id.as_slice()); - let mut statuses: Vec<(Vec, bool)> = Vec::new(); + let mut spent_nullifiers = BTreeSet::>::new(); let mut balance: Option = None; let mut revision: Option = None; let mut keys = BTreeMap::::new(); for (path, key, maybe_element) in proved_key_values { if path == nullifier_path { - statuses.push((key, maybe_element.is_some())); + if !nullifier_keys.contains(&key) { + return Err(Error::Proof(ProofError::CorruptedProof( + "identity create from shielded pool proof contains a nullifier \ + entry that was not requested" + .to_string(), + ))); + } + if maybe_element.is_some() { + spent_nullifiers.insert(key); + } } else if path == balance_path && key == identity_id { let element = maybe_element.ok_or_else(|| { Error::Proof(ProofError::IncompleteProof( @@ -1720,6 +1732,14 @@ impl Drive { } } + // Without absence synthesis an unspent nullifier yields no result entry (or a + // bare absence entry), so each expected nullifier's spend status is its + // membership in the proved-present set. + let statuses: Vec<(Vec, bool)> = nullifier_keys + .iter() + .map(|nf| (nf.clone(), spent_nullifiers.contains(nf))) + .collect(); + // Every funding nullifier must be present (spent) in the post-execution state. for (nf, is_spent) in &statuses { if !is_spent { @@ -3212,10 +3232,10 @@ mod tests { // --- IdentityCreateFromShieldedPool: empty proof returns error. // - // Exercises the STRICT merged-query verify arm: an empty proof cannot satisfy - // `verify_query_with_absence_proof` over the merged {nullifier-tree, identity} query, so the - // verifier must reject (rather than silently accepting). The positive prove→verify roundtrip and - // the padded-proof (extra-branch) rejection are covered by the full-block integration suite. + // Exercises the STRICT merged-query verify arm: an empty proof cannot satisfy the strict + // `verify_query` over the merged {nullifier-tree, identity} query, so the verifier must reject + // (rather than silently accepting). The positive prove→verify roundtrip lives in drive-abci's + // identity_create_from_shielded_pool tests (synthetic-action execution). #[test] fn verify_identity_create_from_shielded_pool_empty_proof_returns_error() { let platform_version = PlatformVersion::latest();