Skip to content

fix(drive): verify identity-create-from-shielded-pool results without unbounded terminal-key queries #3859

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
QuantumExplorer merged 1 commit into from
Jun 11, 2026
Merged

fix(drive): verify identity-create-from-shielded-pool results without unbounded terminal-key queries #3859

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
163 changes: 159 additions & 4 deletions ...validation/state_transition/state_transitions/identity_create_from_shielded_pool/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Vec<u8>> = 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)
);
}
}
6 changes: 4 additions & 2 deletions packages/rs-drive/src/prove/prove_state_transition/v0/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<Vec<u8>> = st.nullifiers();
let mut nf_query = grovedb::Query::new();
nf_query.insert_keys(nullifier_keys);
Expand Down
64 changes: 42 additions & 22 deletions ...ive/src/verify/state_transition/verify_state_transition_was_executed_with_proof/v0/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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<u8>, bool)> = Vec::new();
let mut spent_nullifiers = BTreeSet::<Vec<u8>>::new();
let mut balance: Option<Credits> = None;
let mut revision: Option<Revision> = None;
let mut keys = BTreeMap::<KeyID, IdentityPublicKey>::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(
Expand Down Expand Up @@ -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<u8>, 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 {
Expand Down Expand Up @@ -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();
Expand Down
Loading