diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shielded_withdrawal/tests.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shielded_withdrawal/tests.rs index 2f978377c2..7ecaff6c1d 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shielded_withdrawal/tests.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/shielded_withdrawal/tests.rs @@ -1138,6 +1138,280 @@ mod tests { "withdrawal document should be present (not absent)" ); } + + /// NEGATIVE test for the strict-merged tightening (mirrors + /// `test_shield_from_asset_lock_padded_proof_is_rejected_by_strict_verify`): a proof + /// that carries EXTRA data beyond `{nullifiers, withdrawal-document}` MUST be rejected by + /// the production verifier. + /// + /// The production verify path rebuilds the merged `{nullifiers, withdrawal-document}` + /// query and verifies it with the STRICT `verify_query_with_absence_proof`, whose + /// succinctness check rejects any proof that descends into a subtree (a lower layer) the + /// query did not require. Here we execute a real shielded withdrawal, then have the + /// (honest, but over-broad) prover generate a SUPERSET proof for `{nullifiers, + /// withdrawal-document, + the genesis-populated Pools subtree}` and verify it against the + /// production merged query. The strict verifier must reject it because the proof carries + /// an extra root-level lower layer (`Pools`) the production query never touches. + /// + /// For contrast we also confirm the SUBSET verifier (the looser primitive this change + /// replaced) would have ACCEPTED the same padded proof — demonstrating exactly the hole + /// the strict merged verification closes. + #[test] + fn test_shielded_withdrawal_padded_proof_is_rejected_by_strict_verify() { + use drive::drive::shielded::paths::shielded_credit_pool_nullifiers_path_vec; + use drive::drive::RootTree; + use drive::grovedb::{GroveDb, PathQuery, Query, SizedQuery}; + use drive::query::{SingleDocumentDriveQuery, SingleDocumentDriveQueryContestedStatus}; + + let platform_version = PlatformVersion::latest(); + let platform = setup_platform(); + insert_dummy_encrypted_notes(&platform, 250); + let mut rng = StdRng::seed_from_u64(0); + let pk = get_proving_key(); + + // --- Create keys --- + let sk = SpendingKey::from_bytes([0u8; 32]).unwrap(); + let fvk = FullViewingKey::from(&sk); + let recipient = fvk.address_at(0u32, Scope::External); + let ask = SpendAuthorizingKey::from(&sk); + + // --- Create a spendable note with value 500M --- + let rho_bytes: [u8; 32] = { + let mut b = [0u8; 32]; + b[0] = 1; + b + }; + let rho = Rho::from_bytes(&rho_bytes).unwrap(); + let rseed = RandomSeed::from_bytes([42u8; 32], &rho).unwrap(); + let note = + Note::from_parts(recipient, NoteValue::from_raw(500_000_000), rho, rseed).unwrap(); + + // --- Build commitment tree and get anchor + merkle path --- + let cmx = ExtractedNoteCommitment::from(note.commitment()); + let mut tree = ClientMemoryCommitmentTree::new(100); + tree.append(cmx.to_bytes(), Retention::Marked).unwrap(); + tree.checkpoint(0u32).unwrap(); + let anchor = tree.anchor().unwrap(); + let merkle_path = tree.witness(Position::from(0u64), 0).unwrap().unwrap(); + + // --- Build bundle: spend 500M -> output 5K (value_balance = 499,995,000) --- + let mut builder = Builder::::new(BundleType::DEFAULT, anchor); + builder.add_spend(fvk.clone(), note, merkle_path).unwrap(); + builder + .add_output(None, recipient, NoteValue::from_raw(5_000), [0u8; 36]) + .unwrap(); + + let (unauthorized, _) = builder.build::(&mut rng).unwrap().unwrap(); + + let output_script = create_output_script(); + let unshielding_amount = 499_995_000u64; + let extra_sighash_data = dpp::shielded::shielded_withdrawal_extra_sighash_data( + output_script.as_bytes(), + unshielding_amount, + 1, + Pooling::Never, + ); + let bundle_commitment: [u8; 32] = unauthorized.commitment().into(); + let sighash = compute_platform_sighash(&bundle_commitment, &extra_sighash_data); + + let proven = unauthorized.create_proof(pk, &mut rng).unwrap(); + let bundle = proven.apply_signatures(rng, sighash, &[ask]).unwrap(); + + let (actions, value_balance, anchor_bytes, proof_bytes, binding_sig) = + serialize_authorized_bundle_i64(&bundle); + assert_eq!(value_balance, 499_995_000); + + insert_anchor_into_state(&platform, &anchor_bytes); + set_pool_total_balance(&platform, 500_000_000); + + let transition = create_shielded_withdrawal_transition( + actions, + value_balance as u64, + anchor_bytes, + proof_bytes, + binding_sig, + 1, + Pooling::Never, + output_script.clone(), + ); + + let transition_bytes = transition + .serialize_to_bytes() + .expect("should serialize transition"); + + let platform_state = platform.state.load(); + let transaction = platform.drive.grove.start_transaction(); + + let processing_result = platform + .platform + .process_raw_state_transitions( + &vec![transition_bytes], + &platform_state, + &BlockInfo::default(), + &transaction, + platform_version, + false, + None, + ) + .expect("expected to process state transition"); + + assert_matches!( + processing_result.execution_results().as_slice(), + [StateTransitionExecutionResult::SuccessfulExecution { .. }] + ); + + platform + .drive + .grove + .commit_transaction(transaction) + .unwrap() + .expect("expected to commit transaction"); + + // --- Reconstruct the PRODUCTION merged query exactly as the verifier does --- + // {nullifiers} ∪ {withdrawal-document}, each with cleared limits, then a limit that + // can never truncate the legitimate result set. + let StateTransition::ShieldedWithdrawal(ref st) = transition else { + unreachable!(); + }; + let nullifier_keys: Vec> = st.nullifiers(); + + // Compute withdrawal document ID deterministically (same as prove/verify sides). + let first_nullifier = nullifier_keys + .first() + .expect("should have at least one nullifier"); + let mut entropy = Vec::new(); + entropy.extend_from_slice(first_nullifier); + entropy.extend_from_slice(output_script.as_bytes()); + let document_id = Document::generate_document_id_v0( + &withdrawals_contract::ID, + &withdrawals_contract::OWNER_ID, + withdrawal::NAME, + &entropy, + ); + + let mut nf_query = Query::new(); + nf_query.insert_keys(nullifier_keys); + let nullifier_pq = PathQuery::new( + shielded_credit_pool_nullifiers_path_vec(), + SizedQuery::new(nf_query, None, None), + ); + + let doc_query = SingleDocumentDriveQuery { + contract_id: withdrawals_contract::ID.to_buffer(), + document_type_name: withdrawal::NAME.to_string(), + document_type_keeps_history: false, + document_id: document_id.to_buffer(), + block_time_ms: None, + contested_status: SingleDocumentDriveQueryContestedStatus::NotContested, + }; + let mut doc_pq = doc_query + .construct_path_query(platform_version) + .expect("construct doc path query"); + doc_pq.query.limit = None; + + let mut production_pq = PathQuery::merge( + vec![&nullifier_pq, &doc_pq], + &platform_version.drive.grove_version, + ) + .expect("merge production query"); + production_pq.query.limit = Some(u16::MAX); + + // Sanity: an HONEST proof for the production query verifies strictly (liveness). + let honest_proof = platform + .drive + .grove_get_proved_path_query( + &production_pq, + None, + &mut vec![], + &platform_version.drive, + ) + .expect("honest production proof"); + GroveDb::verify_query_with_absence_proof( + &honest_proof, + &production_pq, + &platform_version.drive.grove_version, + ) + .expect("strict verify of honest production proof must succeed"); + + // --- Build a SUPERSET (padded) proof: {nullifiers, document, + an extra subtree} --- + // Pad by ALSO descending the genesis-populated `Pools` root subtree, which the + // production query never touches; the padded proof carries an extra root-level layer. + let mut pools_top = Query::new(); + pools_top.insert_key(vec![RootTree::Pools as u8]); + pools_top.set_subquery(Query::new_range_full()); + let pools_pq = PathQuery::new(vec![], SizedQuery::new(pools_top, None, None)); + + let mut superset_pq = PathQuery::merge( + vec![&nullifier_pq, &doc_pq, &pools_pq], + &platform_version.drive.grove_version, + ) + .expect("merge superset query"); + superset_pq.query.limit = Some(u16::MAX); + + let padded_proof = platform + .drive + .grove_get_proved_path_query( + &superset_pq, + None, + &mut vec![], + &platform_version.drive, + ) + .expect("padded superset proof"); + + // The STRICT verifier (production behavior) MUST reject the padded proof. + let strict_result = GroveDb::verify_query_with_absence_proof( + &padded_proof, + &production_pq, + &platform_version.drive.grove_version, + ); + assert!( + strict_result.is_err(), + "strict verifier must reject a proof padded with an extra subtree layer, got {:?}", + strict_result + ); + + // And the PRODUCTION entry point — the dispatch site this change actually + // rewrote — MUST reject the padded proof too. Asserting only against the GroveDB + // primitive above would stay green if the ShieldedWithdrawal arm regressed to + // rebuild a different merged query or fall back to the subset verifier; routing the + // padded proof through `Drive::verify_state_transition_was_executed_with_proof` + // locks the real code path into the test. + let withdrawals_data_contract = Arc::new( + load_system_data_contract(SystemDataContract::Withdrawals, platform_version) + .expect("should load withdrawals contract"), + ); + let production_result = Drive::verify_state_transition_was_executed_with_proof( + &transition, + &BlockInfo::default(), + &padded_proof, + &|id| { + if *id == withdrawals_contract::ID { + Ok(Some(Arc::clone(&withdrawals_data_contract))) + } else { + Ok(None) + } + }, + platform_version, + ); + assert!( + production_result.is_err(), + "production shielded withdrawal verifier must reject a padded proof, got {:?}", + production_result + ); + + // Contrast: the SUBSET verifier (the looser primitive this change replaced) tolerates + // the extra layer and ACCEPTS the same padded proof — the exact hole now closed. + let subset_result = GroveDb::verify_subset_query_with_absence_proof( + &padded_proof, + &production_pq, + &platform_version.drive.grove_version, + ); + assert!( + subset_result.is_ok(), + "subset verifier was expected to tolerate the padded proof, got {:?}", + subset_result + ); + } } mod credit_conservation { diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/unshield/tests.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/unshield/tests.rs index 3fe96a5e19..cf86527d95 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/unshield/tests.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/unshield/tests.rs @@ -968,5 +968,249 @@ mod tests { output_address ); } + + /// NEGATIVE test for the strict-merged tightening (mirrors + /// `test_shield_from_asset_lock_padded_proof_is_rejected_by_strict_verify`): a proof + /// that carries EXTRA data beyond `{nullifiers, output-address}` MUST be rejected by the + /// production verifier. + /// + /// The production verify path rebuilds the merged `{nullifiers, output-address}` query + /// and verifies it with the STRICT `verify_query_with_absence_proof`, whose succinctness + /// check rejects any proof that descends into a subtree (a lower layer) the query did not + /// require. Here we execute a real unshield, then have the (honest, but over-broad) + /// prover generate a SUPERSET proof for `{nullifiers, output-address, + the + /// genesis-populated Pools subtree}` and verify it against the production merged query. + /// The strict verifier must reject it because the proof carries an extra root-level lower + /// layer (`Pools`) the production query never touches. + /// + /// For contrast we also confirm the SUBSET verifier (the looser primitive this change + /// replaced) would have ACCEPTED the same padded proof — demonstrating exactly the hole + /// the strict merged verification closes. + #[test] + fn test_unshield_padded_proof_is_rejected_by_strict_verify() { + use drive::drive::shielded::paths::shielded_credit_pool_nullifiers_path_vec; + use drive::drive::RootTree; + use drive::grovedb::{GroveDb, PathQuery, Query, SizedQuery}; + + let platform_version = PlatformVersion::latest(); + let platform = setup_platform(); + insert_dummy_encrypted_notes(&platform, 250); + let mut rng = StdRng::seed_from_u64(0); + let pk = get_proving_key(); + + let spend_amount = 500_000_000u64; + let output_amount = 5_000u64; + + // --- Create keys --- + let sk = SpendingKey::from_bytes([0u8; 32]).unwrap(); + let fvk = FullViewingKey::from(&sk); + let recipient = fvk.address_at(0u32, Scope::External); + let ask = SpendAuthorizingKey::from(&sk); + + // --- Create a spendable note --- + let rho_bytes: [u8; 32] = { + let mut b = [0u8; 32]; + b[0] = 1; + b + }; + let rho = Rho::from_bytes(&rho_bytes).unwrap(); + let rseed = RandomSeed::from_bytes([42u8; 32], &rho).unwrap(); + let note = + Note::from_parts(recipient, NoteValue::from_raw(spend_amount), rho, rseed).unwrap(); + + // --- Build commitment tree and get anchor + merkle path --- + let cmx = ExtractedNoteCommitment::from(note.commitment()); + let mut tree = ClientMemoryCommitmentTree::new(100); + tree.append(cmx.to_bytes(), Retention::Marked).unwrap(); + tree.checkpoint(0u32).unwrap(); + let anchor = tree.anchor().unwrap(); + let merkle_path = tree.witness(Position::from(0u64), 0).unwrap().unwrap(); + + // --- Build bundle: spend 500M -> output 5K (value_balance = 499,995,000) --- + let mut builder = Builder::::new(BundleType::DEFAULT, anchor); + builder.add_spend(fvk.clone(), note, merkle_path).unwrap(); + builder + .add_output( + None, + recipient, + NoteValue::from_raw(output_amount), + [0u8; 36], + ) + .unwrap(); + + let (unauthorized, _) = builder.build::(&mut rng).unwrap().unwrap(); + + let output_address = create_output_address(); + let unshielding_amount = 499_995_000u64; + let extra_sighash_data = dpp::shielded::unshield_extra_sighash_data( + &output_address.to_bytes(), + unshielding_amount, + ); + let bundle_commitment: [u8; 32] = unauthorized.commitment().into(); + let sighash = compute_platform_sighash(&bundle_commitment, &extra_sighash_data); + + let proven = unauthorized.create_proof(pk, &mut rng).unwrap(); + let bundle = proven.apply_signatures(rng, sighash, &[ask]).unwrap(); + + let (actions, value_balance, anchor_bytes, proof_bytes, binding_sig) = + serialize_authorized_bundle_i64(&bundle); + assert_eq!(value_balance, 499_995_000); + + insert_anchor_into_state(&platform, &anchor_bytes); + set_pool_total_balance(&platform, 500_000_000); + + let transition = create_unshield_transition( + output_address.clone(), + actions, + value_balance as u64, + anchor_bytes, + proof_bytes, + binding_sig, + ); + + let transition_bytes = transition + .serialize_to_bytes() + .expect("should serialize transition"); + + let platform_state = platform.state.load(); + let transaction = platform.drive.grove.start_transaction(); + + let processing_result = platform + .platform + .process_raw_state_transitions( + &vec![transition_bytes], + &platform_state, + &BlockInfo::default(), + &transaction, + platform_version, + false, + None, + ) + .expect("expected to process state transition"); + + assert_matches!( + processing_result.execution_results().as_slice(), + [StateTransitionExecutionResult::SuccessfulExecution { .. }] + ); + + platform + .drive + .grove + .commit_transaction(transaction) + .unwrap() + .expect("expected to commit transaction"); + + // --- Reconstruct the PRODUCTION merged query exactly as the verifier does --- + // {nullifiers} ∪ {output address}, each with cleared limits, then a limit that can + // never truncate the legitimate result set. + let StateTransition::Unshield(ref st) = transition else { + unreachable!(); + }; + let nullifier_keys: Vec> = st.nullifiers(); + + let mut nf_query = Query::new(); + nf_query.insert_keys(nullifier_keys); + let nullifier_pq = PathQuery::new( + shielded_credit_pool_nullifiers_path_vec(), + SizedQuery::new(nf_query, None, None), + ); + + let mut address_pq = + Drive::balances_for_clear_addresses_query(std::iter::once(&output_address)); + address_pq.query.limit = None; + + let mut production_pq = PathQuery::merge( + vec![&nullifier_pq, &address_pq], + &platform_version.drive.grove_version, + ) + .expect("merge production query"); + production_pq.query.limit = Some(u16::MAX); + + // Sanity: an HONEST proof for the production query verifies strictly (liveness). + let honest_proof = platform + .drive + .grove_get_proved_path_query( + &production_pq, + None, + &mut vec![], + &platform_version.drive, + ) + .expect("honest production proof"); + GroveDb::verify_query_with_absence_proof( + &honest_proof, + &production_pq, + &platform_version.drive.grove_version, + ) + .expect("strict verify of honest production proof must succeed"); + + // --- Build a SUPERSET (padded) proof: {nullifiers, address, + an extra subtree} --- + // Pad by ALSO descending the genesis-populated `Pools` root subtree, which the + // production query never touches; the padded proof carries an extra root-level layer. + let mut pools_top = Query::new(); + pools_top.insert_key(vec![RootTree::Pools as u8]); + pools_top.set_subquery(Query::new_range_full()); + let pools_pq = PathQuery::new(vec![], SizedQuery::new(pools_top, None, None)); + + let mut superset_pq = PathQuery::merge( + vec![&nullifier_pq, &address_pq, &pools_pq], + &platform_version.drive.grove_version, + ) + .expect("merge superset query"); + superset_pq.query.limit = Some(u16::MAX); + + let padded_proof = platform + .drive + .grove_get_proved_path_query( + &superset_pq, + None, + &mut vec![], + &platform_version.drive, + ) + .expect("padded superset proof"); + + // The STRICT verifier (production behavior) MUST reject the padded proof. + let strict_result = GroveDb::verify_query_with_absence_proof( + &padded_proof, + &production_pq, + &platform_version.drive.grove_version, + ); + assert!( + strict_result.is_err(), + "strict verifier must reject a proof padded with an extra subtree layer, got {:?}", + strict_result + ); + + // And the PRODUCTION entry point — the dispatch site this change actually + // rewrote — MUST reject the padded proof too. Asserting only against the GroveDB + // primitive above would stay green if the Unshield arm regressed to rebuild a + // different merged query or fall back to the subset verifier; routing the padded + // proof through `Drive::verify_state_transition_was_executed_with_proof` locks the + // real code path into the test. + let production_result = Drive::verify_state_transition_was_executed_with_proof( + &transition, + &BlockInfo::default(), + &padded_proof, + &|_| Ok(None), + platform_version, + ); + assert!( + production_result.is_err(), + "production unshield verifier must reject a padded proof, got {:?}", + production_result + ); + + // Contrast: the SUBSET verifier (the looser primitive this change replaced) tolerates + // the extra layer and ACCEPTS the same padded proof — the exact hole now closed. + let subset_result = GroveDb::verify_subset_query_with_absence_proof( + &padded_proof, + &production_pq, + &platform_version.drive.grove_version, + ); + assert!( + subset_result.is_ok(), + "subset verifier was expected to tolerate the padded proof, got {:?}", + subset_result + ); + } } } 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 3b8caf885c..72af4fbd4d 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 @@ -1133,18 +1133,100 @@ impl Drive { Ok((root_hash, VerifiedAddressInfos(balances))) } StateTransition::Unshield(st) => { + use crate::drive::shielded::paths::shielded_credit_pool_nullifiers_path_vec; use dpp::state_transition::proof_result::StateTransitionProofResult::VerifiedShieldedNullifiersWithAddressInfos; use dpp::state_transition::unshield_transition::accessors::UnshieldTransitionAccessorsV0; + use grovedb::Element; let nullifier_keys: Vec> = st.nullifiers(); - let (root_hash_nf, statuses) = Drive::verify_shielded_nullifiers( + // Reconstruct the prove side's merged query — nullifier spend-status ∪ + // output-address balance — and verify it strictly. See + // `verify_merged_query_strict` for why a single strict merged verify is + // sound and rejects proofs padded with extra subtree branches. + let mut nf_query = grovedb::Query::new(); + nf_query.insert_keys(nullifier_keys); + let nullifier_pq = grovedb::PathQuery::new( + shielded_credit_pool_nullifiers_path_vec(), + grovedb::SizedQuery::new(nf_query, None, None), + ); + + let address_pq = + Drive::balances_for_clear_addresses_query(std::iter::once(st.output_address())); + + let (root_hash, proved_key_values) = Self::verify_merged_query_strict( proof, - &nullifier_keys, - true, + vec![nullifier_pq, address_pq], platform_version, )?; + // Partition the proved key/values by path: entries under the nullifiers tree are + // spend statuses, entries under the clear-address pool are address balances. + let nullifiers_path = shielded_credit_pool_nullifiers_path_vec(); + let addresses_path = Drive::clear_addresses_path(); + + let mut statuses: Vec<(Vec, bool)> = Vec::new(); + let mut balances: BTreeMap> = + BTreeMap::new(); + + for (path, key, element) in proved_key_values { + if path == nullifiers_path { + // A present element means the nullifier is spent; absence means unspent. + statuses.push((key, element.is_some())); + } else if path == addresses_path { + // Mirror `verify_addresses_infos_v0`: reconstruct the address from the key + // and decode the `ItemWithSumItem` (nonce, balance) element. + let address = PlatformAddress::from_bytes(&key).map_err(|e| { + Error::Proof(ProofError::CorruptedProof(format!( + "failed to deserialize output PlatformAddress: {}", + e + ))) + })?; + + let balance_info = element + .map(|element| { + let Element::ItemWithSumItem(nonce_vec, balance_i64, _) = element + else { + return Err(Error::Proof(ProofError::CorruptedProof( + "expected an item with sum item element".to_string(), + ))); + }; + + let nonce_bytes: [u8; 4] = nonce_vec.try_into().map_err(|_| { + Error::Proof(ProofError::IncorrectValueSize( + "nonce should be 4 bytes", + )) + })?; + let nonce = AddressNonce::from_be_bytes(nonce_bytes); + + if balance_i64 < 0 { + return Err(Error::Proof(ProofError::CorruptedProof( + "balance cannot be negative".to_string(), + ))); + } + + Ok((nonce, balance_i64 as Credits)) + }) + .transpose()?; + + // Mirror ShieldedWithdrawal's singleton-subtree invariant: the + // address sub-query targets exactly one key, so a second entry under + // the clear-address pool is a malformed proof, not last-write-wins. + if balances.contains_key(&address) { + return Err(Error::Proof(ProofError::CorruptedProof( + "unshield proof contained more than one output-address entry" + .to_string(), + ))); + } + balances.insert(address, balance_info); + } else { + return Err(Error::Proof(ProofError::CorruptedProof( + "unshield proof contained an entry outside the nullifier and address subtrees".to_string(), + ))); + } + } + + // Every nullifier must be present and marked spent. for (nf, is_spent) in &statuses { if !is_spent { return Err(Error::Proof(ProofError::IncorrectProof(format!( @@ -1154,25 +1236,8 @@ impl Drive { } } - let (root_hash_addr, balances): ( - RootHash, - BTreeMap>, - ) = Drive::verify_addresses_infos( - proof, - std::iter::once(st.output_address()), - true, - platform_version, - )?; - - if root_hash_nf != root_hash_addr { - return Err(Error::Proof(ProofError::CorruptedProof( - "unshield proof root hashes do not match between nullifiers and address" - .to_string(), - ))); - } - Ok(( - root_hash_nf, + root_hash, VerifiedShieldedNullifiersWithAddressInfos(statuses, balances), )) } @@ -1202,31 +1267,18 @@ impl Drive { Ok((root_hash, VerifiedShieldedNullifiers(statuses))) } StateTransition::ShieldedWithdrawal(st) => { + use crate::drive::shielded::paths::shielded_credit_pool_nullifiers_path_vec; use dpp::data_contracts::withdrawals_contract; use dpp::data_contracts::withdrawals_contract::v1::document_types::withdrawal; + use dpp::document::serialization_traits::DocumentPlatformConversionMethodsV0; use dpp::document::Document; use dpp::state_transition::proof_result::StateTransitionProofResult::VerifiedShieldedNullifiersWithWithdrawalDocument; use dpp::state_transition::shielded_withdrawal_transition::accessors::ShieldedWithdrawalTransitionAccessorsV0; + use grovedb::Element; let nullifier_keys: Vec> = st.nullifiers(); - let (root_hash_nf, statuses) = Drive::verify_shielded_nullifiers( - proof, - &nullifier_keys, - true, - platform_version, - )?; - - for (nf, is_spent) in &statuses { - if !is_spent { - return Err(Error::Proof(ProofError::IncorrectProof(format!( - "nullifier {} was not found as spent in the shielded withdrawal proof", - hex::encode(nf) - )))); - } - } - - // Compute withdrawal document ID deterministically (same as prove side) + // Compute withdrawal document ID deterministically (same as prove side). let first_nullifier = nullifier_keys.first().ok_or_else(|| { Error::Proof(ProofError::InvalidTransition( "shielded withdrawal has no nullifiers".to_string(), @@ -1268,25 +1320,88 @@ impl Drive { contested_status: SingleDocumentDriveQueryContestedStatus::NotContested, }; - let (root_hash_doc, maybe_doc) = - doc_query.verify_proof(true, proof, document_type, platform_version)?; + // Reconstruct the prove side's merged query — nullifier spend-status ∪ + // withdrawal document — and verify it strictly. See + // `verify_merged_query_strict` for why a single strict merged verify is + // sound and rejects proofs padded with extra subtree branches. + let mut nf_query = grovedb::Query::new(); + nf_query.insert_keys(nullifier_keys); + let nullifier_pq = grovedb::PathQuery::new( + shielded_credit_pool_nullifiers_path_vec(), + grovedb::SizedQuery::new(nf_query, None, None), + ); - if root_hash_nf != root_hash_doc { - return Err(Error::Proof(ProofError::CorruptedProof( - "shielded withdrawal proof root hashes do not match between nullifiers and document" - .to_string(), - ))); + let doc_pq = doc_query.construct_path_query(platform_version)?; + let document_path = doc_pq.path.clone(); + + let (root_hash, proved_key_values) = Self::verify_merged_query_strict( + proof, + vec![nullifier_pq, doc_pq], + platform_version, + )?; + + // Partition the proved key/values by path: entries under the nullifiers tree are + // spend statuses; the single entry under the withdrawal-document tree is the + // proven document. + let nullifiers_path = shielded_credit_pool_nullifiers_path_vec(); + + let mut statuses: Vec<(Vec, bool)> = Vec::new(); + let mut document_element: Option> = None; + + for (path, key, element) in proved_key_values { + if path == nullifiers_path { + statuses.push((key, element.is_some())); + } else if path == document_path { + if document_element.is_some() { + return Err(Error::Proof(ProofError::CorruptedProof( + "shielded withdrawal proof contained more than one withdrawal document".to_string(), + ))); + } + document_element = Some(element); + } else { + return Err(Error::Proof(ProofError::CorruptedProof( + "shielded withdrawal proof contained an entry outside the nullifier and document subtrees".to_string(), + ))); + } + } + + // Every nullifier must be present and marked spent. + for (nf, is_spent) in &statuses { + if !is_spent { + return Err(Error::Proof(ProofError::IncorrectProof(format!( + "nullifier {} was not found as spent in the shielded withdrawal proof", + hex::encode(nf) + )))); + } } - let doc = maybe_doc.ok_or_else(|| { + let document_element = document_element.ok_or_else(|| { Error::Proof(ProofError::CorruptedProof( - "shielded withdrawal was executed but withdrawal document is missing from proof".to_string(), + "shielded withdrawal document key absent from proof".to_string(), )) })?; + + let doc = match document_element { + Some(Element::Item(serialized, _)) => Document::from_bytes( + serialized.as_slice(), + document_type, + platform_version, + )?, + Some(_) => { + return Err(Error::Proof(ProofError::CorruptedProof( + "expected an item element for withdrawal document".to_string(), + ))); + } + None => { + return Err(Error::Proof(ProofError::CorruptedProof( + "shielded withdrawal was executed but withdrawal document is missing from proof".to_string(), + ))); + } + }; let documents = BTreeMap::from([(document_id, Some(doc))]); Ok(( - root_hash_nf, + root_hash, VerifiedShieldedNullifiersWithWithdrawalDocument(statuses, documents), )) } @@ -1323,47 +1438,20 @@ impl Drive { match surplus_output { Some(surplus_address) => { - // The prove side merged the outpoint sub-query with the surplus-address - // balance sub-query into a SINGLE multi-root proof (clearing each sub-query - // limit before the merge, because `PathQuery::merge` rejects limited - // sub-queries). Reconstruct the byte-identical merged query here and verify - // it STRICTLY: the strict verifier accepts a proof that matches the merged - // query exactly and REJECTS any proof carrying extra branches, so a - // malicious/buggy prover cannot pad the proof with unrelated data. - let mut outpoint_pq = outpoint_pq; - outpoint_pq.query.limit = None; - - let mut address_pq = Drive::balances_for_clear_addresses_query( + // Reconstruct the prove side's merged query — asset-lock outpoint ∪ + // surplus-address balance — and verify it strictly. See + // `verify_merged_query_strict` for why a single strict merged verify + // is sound and rejects proofs padded with extra subtree branches. + let address_pq = Drive::balances_for_clear_addresses_query( std::iter::once(surplus_address), ); - address_pq.query.limit = None; - let mut merged_pq = grovedb::PathQuery::merge( - vec![&outpoint_pq, &address_pq], - &platform_version.drive.grove_version, + let (root_hash, proved_key_values) = Self::verify_merged_query_strict( + proof, + vec![outpoint_pq, address_pq], + platform_version, )?; - // `verify_query_with_absence_proof` (the STRICT verifier) requires a limit - // to be set, but `PathQuery::merge` leaves the merged limit at `None`. The - // merged query targets a fixed, tiny set of explicit keys ({outpoint} ∪ - // {surplus address}), so we set a limit that can never be exhausted by the - // legitimate result set. This is load-bearing for soundness: the succinctness - // check that rejects extra proof layers runs per-layer AFTER that layer's - // result loop, and the result loop only breaks early once the limit hits 0. - // A limit smaller than the real result count could break before every layer's - // succinctness check runs (falsely rejecting honest proofs); an unreachable - // limit guarantees every layer is fully traversed and every extra branch is - // caught. The limit does NOT relax the extra-data rejection — that is the - // succinctness check, which is independent of the limit value. - 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, - )?; - // Partition the proved key/values: exactly one entry is the asset-lock // outpoint (36-byte key), every other entry is a surplus-address balance // (21-byte key). Anything else is a corrupted proof. @@ -1507,6 +1595,53 @@ impl Drive { } } } + + /// Reconstruct the prove side's merged multi-root query and verify it STRICTLY. + /// + /// The shielded prove paths merge several sub-queries into a SINGLE multi-root + /// proof. [`grovedb::PathQuery::merge`] rejects sub-queries that still carry a + /// limit, so every sub-query's limit is cleared here before the merge. The strict + /// [`grovedb::GroveDb::verify_query_with_absence_proof`] in turn *requires* a + /// limit, but it must never be exhausted by the legitimate result set: the + /// per-layer succinctness check that rejects extra proof branches runs AFTER that + /// layer's result loop, and the result loop only breaks early once the limit hits + /// 0 — so a limit smaller than the real result count could break before a layer's + /// succinctness check runs and falsely reject an honest proof. Every shielded + /// merged query targets a fixed, tiny set of explicit keys ({nullifiers} plus one + /// address/document/outpoint), so an unreachable `u16::MAX` limit is sound: it + /// guarantees every layer is fully traversed while the (limit-independent) + /// succinctness check still rejects any proof padded with extra subtree branches. + /// + /// Returns the proof root hash and every proved `(path, key, element)` trio, left + /// for the caller to partition against the sub-query paths. + #[allow(clippy::type_complexity)] + fn verify_merged_query_strict( + proof: &[u8], + mut sub_queries: Vec, + platform_version: &PlatformVersion, + ) -> Result< + ( + RootHash, + Vec, + ), + Error, + > { + for sub_query in &mut sub_queries { + sub_query.query.limit = None; + } + + let mut merged_pq = grovedb::PathQuery::merge( + sub_queries.iter().collect(), + &platform_version.drive.grove_version, + )?; + merged_pq.query.limit = Some(u16::MAX); + + Ok(grovedb::GroveDb::verify_query_with_absence_proof( + proof, + &merged_pq, + &platform_version.drive.grove_version, + )?) + } } #[cfg(feature = "server")]