diff --git a/packages/rs-drive/src/verify/contract/verify_contract/v0/mod.rs b/packages/rs-drive/src/verify/contract/verify_contract/v0/mod.rs index e12c9612fda..69db47c079f 100644 --- a/packages/rs-drive/src/verify/contract/verify_contract/v0/mod.rs +++ b/packages/rs-drive/src/verify/contract/verify_contract/v0/mod.rs @@ -42,10 +42,49 @@ impl Drive { contract_id: [u8; 32], platform_version: &PlatformVersion, ) -> Result<(RootHash, Option), Error> { - let path_query = match ( + let keeps_history = contract_known_keeps_history.unwrap_or(false); + + let result = Self::verify_contract_v0_given_history( + proof, + keeps_history, + is_proof_subset, in_multiple_contract_proof_form, - contract_known_keeps_history.unwrap_or_default(), - ) { + contract_id, + platform_version, + ); + + if contract_known_keeps_history.is_none() { + match &result { + Ok((_, Some(_))) => result, + _ => { + tracing::debug!( + ?contract_id, + "retrying contract verification with history enabled" + ); + Self::verify_contract_v0_given_history( + proof, + true, + is_proof_subset, + in_multiple_contract_proof_form, + contract_id, + platform_version, + ) + } + } + } else { + result + } + } + + fn verify_contract_v0_given_history( + proof: &[u8], + keeps_history: bool, + is_proof_subset: bool, + in_multiple_contract_proof_form: bool, + contract_id: [u8; 32], + platform_version: &PlatformVersion, + ) -> Result<(RootHash, Option), Error> { + let path_query = match (in_multiple_contract_proof_form, keeps_history) { (true, true) => Self::fetch_historical_contracts_query(&[contract_id]), (true, false) => Self::fetch_non_historical_contracts_query(&[contract_id]), (false, true) => Self::fetch_contract_with_history_latest_query(contract_id, true), @@ -67,25 +106,7 @@ impl Drive { &platform_version.drive.grove_version, ) }; - let (root_hash, mut proved_key_values) = match result.map_err(Error::from) { - Ok(ok_result) => ok_result, - Err(e) => { - return if contract_known_keeps_history.is_none() { - tracing::debug!(?path_query,error=?e, "retrying contract verification with history enabled"); - // most likely we are trying to prove a historical contract - Self::verify_contract( - proof, - Some(true), - is_proof_subset, - in_multiple_contract_proof_form, - contract_id, - platform_version, - ) - } else { - Err(e) - }; - } - }; + let (root_hash, mut proved_key_values) = result.map_err(Error::from)?; if proved_key_values.is_empty() { return Err(Error::Proof(ProofError::WrongElementCount { expected: 1, @@ -94,7 +115,7 @@ impl Drive { } if proved_key_values.len() == 1 { let (path, key, maybe_element) = proved_key_values.remove(0); - if contract_known_keeps_history.unwrap_or_default() { + if keeps_history { if path != contract_keeping_history_root_path(&contract_id) { return Err(Error::Proof(ProofError::CorruptedProof( "we did not get back an element for the correct path for the historical contract".to_string(), @@ -125,26 +146,9 @@ impl Drive { .map_err(Error::from) }) }) - .transpose(); - match contract { - Ok(contract) => Ok((root_hash, contract)), - Err(e) => { - if contract_known_keeps_history.is_some() { - // just return error - Err(e) - } else { - tracing::debug!(?path_query,error=?e, "retry contract verification with history enabled"); - Self::verify_contract( - proof, - Some(true), - is_proof_subset, - in_multiple_contract_proof_form, - contract_id, - platform_version, - ) - } - } - } + .transpose()?; + + Ok((root_hash, contract)) } else { Err(Error::Proof(ProofError::TooManyElements( "expected one contract id", diff --git a/packages/rs-drive/src/verify/contract/verify_contract_return_serialization/v0/mod.rs b/packages/rs-drive/src/verify/contract/verify_contract_return_serialization/v0/mod.rs index 6ff6d832bba..1eb7beb1ba9 100644 --- a/packages/rs-drive/src/verify/contract/verify_contract_return_serialization/v0/mod.rs +++ b/packages/rs-drive/src/verify/contract/verify_contract_return_serialization/v0/mod.rs @@ -44,10 +44,49 @@ impl Drive { contract_id: [u8; 32], platform_version: &PlatformVersion, ) -> Result { - let path_query = match ( + let keeps_history = contract_known_keeps_history.unwrap_or(false); + + let result = Self::verify_contract_return_serialization_v0_given_history( + proof, + keeps_history, + is_proof_subset, in_multiple_contract_proof_form, - contract_known_keeps_history.unwrap_or_default(), - ) { + contract_id, + platform_version, + ); + + if contract_known_keeps_history.is_none() { + match &result { + Ok((_, Some(_))) => result, + _ => { + tracing::debug!( + ?contract_id, + "retrying contract verification with history enabled" + ); + Self::verify_contract_return_serialization_v0_given_history( + proof, + true, + is_proof_subset, + in_multiple_contract_proof_form, + contract_id, + platform_version, + ) + } + } + } else { + result + } + } + + fn verify_contract_return_serialization_v0_given_history( + proof: &[u8], + keeps_history: bool, + is_proof_subset: bool, + in_multiple_contract_proof_form: bool, + contract_id: [u8; 32], + platform_version: &PlatformVersion, + ) -> Result { + let path_query = match (in_multiple_contract_proof_form, keeps_history) { (true, true) => Self::fetch_historical_contracts_query(&[contract_id]), (true, false) => Self::fetch_non_historical_contracts_query(&[contract_id]), (false, true) => Self::fetch_contract_with_history_latest_query(contract_id, true), @@ -69,25 +108,7 @@ impl Drive { &platform_version.drive.grove_version, ) }; - let (root_hash, mut proved_key_values) = match result.map_err(Error::from) { - Ok(ok_result) => ok_result, - Err(e) => { - return if contract_known_keeps_history.is_none() { - tracing::debug!(?path_query,error=?e, "retrying contract verification with history enabled"); - // most likely we are trying to prove a historical contract - Self::verify_contract_return_serialization_v0( - proof, - Some(true), - is_proof_subset, - in_multiple_contract_proof_form, - contract_id, - platform_version, - ) - } else { - Err(e) - }; - } - }; + let (root_hash, mut proved_key_values) = result.map_err(Error::from)?; if proved_key_values.is_empty() { return Err(Error::Proof(ProofError::WrongElementCount { expected: 1, @@ -96,7 +117,7 @@ impl Drive { } if proved_key_values.len() == 1 { let (path, key, maybe_element) = proved_key_values.remove(0); - if contract_known_keeps_history.unwrap_or_default() { + if keeps_history { if path != contract_keeping_history_root_path(&contract_id) { return Err(Error::Proof(ProofError::CorruptedProof( "we did not get back an element for the correct path for the historical contract".to_string(), @@ -134,26 +155,9 @@ impl Drive { )) }) }) - .transpose(); - match contract { - Ok(contract) => Ok((root_hash, contract)), - Err(e) => { - if contract_known_keeps_history.is_some() { - // just return error - Err(e) - } else { - tracing::debug!(?path_query,error=?e, "retry contract verification with history enabled"); - Self::verify_contract_return_serialization_v0( - proof, - Some(true), - is_proof_subset, - in_multiple_contract_proof_form, - contract_id, - platform_version, - ) - } - } - } + .transpose()?; + + Ok((root_hash, contract)) } else { Err(Error::Proof(ProofError::TooManyElements( "expected one contract id", diff --git a/packages/rs-drive/tests/query_tests.rs b/packages/rs-drive/tests/query_tests.rs index 5d6f1590ccf..333a2a8d4d9 100644 --- a/packages/rs-drive/tests/query_tests.rs +++ b/packages/rs-drive/tests/query_tests.rs @@ -59,6 +59,8 @@ use base64::Engine; #[cfg(feature = "server")] use dpp::block::block_info::BlockInfo; use dpp::data_contract::accessors::v0::DataContractV0Getters; +use dpp::data_contract::accessors::v0::DataContractV0Setters; +use dpp::data_contract::config::v0::DataContractConfigSettersV0; use dpp::data_contract::config::v1::DataContractConfigSettersV1; use dpp::data_contract::conversion::value::v0::DataContractValueConversionMethodsV0; use dpp::data_contract::document_type::methods::DocumentTypeV0Methods; @@ -918,6 +920,14 @@ pub fn setup_withdrawal_tests( #[cfg(feature = "server")] /// Sets up the References contract to test queries on. pub fn setup_references_tests(_count: u32, _seed: u64) -> (Drive, DataContract) { + setup_references_tests_with_keeps_history(_count, _seed, false) +} + +pub fn setup_references_tests_with_keeps_history( + _count: u32, + _seed: u64, + keeps_history: bool, +) -> (Drive, DataContract) { let drive = setup_drive(Some(DriveConfig::default())); let db_transaction = drive.grove.start_transaction(); @@ -939,7 +949,9 @@ pub fn setup_references_tests(_count: u32, _seed: u64) -> (Drive, DataContract) "tests/supporting_files/contract/references/references_with_contract_history.json", None, None, - None::, + Some(|contract: &mut DataContract| { + contract.config_mut().set_keeps_history(keeps_history); + }), Some(&db_transaction), None, ); @@ -4687,6 +4699,119 @@ mod tests { ); } + #[test] + fn test_contract_keeps_history_verify_with_unknown_history_flag() { + // Regression test: when contract_known_keeps_history is None, + // verification must still succeed for historical contracts. + let (drive, contract) = setup_references_tests_with_keeps_history(10, 3334, true); + let platform_version = PlatformVersion::latest(); + + // Apply an update so the contract has an actual history entry and latest historical path. + let mut latest_contract = contract.clone(); + latest_contract.set_version(contract.version() + 1); + drive + .apply_contract( + &latest_contract, + BlockInfo { + time_ms: 1, + ..Default::default() + }, + true, + StorageFlags::optional_default_as_cow(), + None, + platform_version, + ) + .expect("contract update should be applied"); + + let root_hash = drive + .grove + .root_hash(None, &platform_version.drive.grove_version) + .unwrap() + .expect("there is always a root hash"); + + let contract_proof = drive + .prove_contract(latest_contract.id().into_buffer(), None, platform_version) + .expect("expected to get proof"); + + // Test 1: None (unknown) - verification must succeed, and may use retry behavior. + let (proof_root_hash, proof_contract) = Drive::verify_contract( + contract_proof.as_slice(), + None, + false, + false, + latest_contract.id().into_buffer(), + platform_version, + ) + .expect("verification with None should succeed"); + assert_eq!(root_hash, proof_root_hash); + assert_eq!( + latest_contract, + proof_contract.expect("contract exists and was updated — None retry must return it") + ); + + // Test 2: Some(true) - direct historical, must succeed since proof was generated + // for a historical contract + let (proof_root_hash_2, proof_contract_2) = Drive::verify_contract( + contract_proof.as_slice(), + Some(true), + false, + false, + latest_contract.id().into_buffer(), + platform_version, + ) + .expect("verification with Some(true) should succeed for historical contract"); + assert_eq!(root_hash, proof_root_hash_2); + assert_eq!( + latest_contract, + proof_contract_2.expect("expected contract with explicit history flag") + ); + + // Test 3: Some(false) - explicit non-historical contract must verify to existing value. + let (non_historical_drive, non_historical_contract) = + setup_references_tests_with_keeps_history(10, 3334, false); + let non_historical_root_hash = non_historical_drive + .grove + .root_hash(None, &platform_version.drive.grove_version) + .unwrap() + .expect("there is always a root hash"); + let non_historical_proof = non_historical_drive + .prove_contract( + non_historical_contract.id().into_buffer(), + None, + platform_version, + ) + .expect("expected to get proof"); + let (proof_root_hash_3, proof_contract_3) = Drive::verify_contract( + non_historical_proof.as_slice(), + Some(false), + false, + false, + non_historical_contract.id().into_buffer(), + platform_version, + ) + .expect("verification with Some(false) should return the existing contract"); + assert_eq!(non_historical_root_hash, proof_root_hash_3); + assert_eq!( + non_historical_contract, + proof_contract_3.expect("expected contract with explicit non-history flag") + ); + + // Test 4: verify_contract_return_serialization with None + let (proof_root_hash_4, proof_contract_4) = Drive::verify_contract_return_serialization( + contract_proof.as_slice(), + None, + false, + false, + latest_contract.id().into_buffer(), + platform_version, + ) + .expect("return_serialization with None should succeed for historical contract"); + assert_eq!(root_hash, proof_root_hash_4); + let (deserialized, _bytes) = + proof_contract_4.expect("contract exists — return_serialization must return it"); + assert_eq!(latest_contract, deserialized); + } + #[cfg(feature = "server")] #[test] fn test_dpns_query_first_version() {