From d7c34e2bb2667605344d7dd437b3e01f0346bf76 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Wed, 22 Apr 2026 06:19:15 +0800 Subject: [PATCH 1/2] fix(dpp): read decryption bounded-key from its own property in V0 parser `DataContractConfigV0::get_contract_configuration_properties_v0` read both `requires_identity_encryption_bounded_key` and `requires_identity_decryption_bounded_key` from the same map key (`REQUIRES_IDENTITY_ENCRYPTION_BOUNDED_KEY`), so V0 configs parsed from a property map could never set the two fields to different values. V1's parser is correct; V0 had a copy-paste typo dating back to the initial contract bounds feature in #1358. The V0 dispatcher is currently dead (no active callers; parent is marked `TODO: Remove, it's not using`), so the fix cannot alter any already-ingested contract state or consensus hashes. Added regression tests covering distinct values, absence, and decryption-only. --- .../rs-dpp/src/data_contract/config/mod.rs | 65 +++++++++++++++++++ .../rs-dpp/src/data_contract/config/v0/mod.rs | 2 +- 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/packages/rs-dpp/src/data_contract/config/mod.rs b/packages/rs-dpp/src/data_contract/config/mod.rs index 175479456f1..eef81776206 100644 --- a/packages/rs-dpp/src/data_contract/config/mod.rs +++ b/packages/rs-dpp/src/data_contract/config/mod.rs @@ -583,4 +583,69 @@ mod tests { } } } + + mod get_contract_configuration_properties { + use super::*; + use crate::data_contract::config::property::{ + REQUIRES_IDENTITY_DECRYPTION_BOUNDED_KEY, REQUIRES_IDENTITY_ENCRYPTION_BOUNDED_KEY, + }; + use platform_value::Value; + use std::collections::BTreeMap; + + /// Regression test for a copy-paste bug where V0 parsing read both + /// encryption and decryption bounded-key fields from the same map key + /// (`requiresIdentityEncryptionBoundedKey`), making it impossible to + /// set them to different values via the map-based parser. + #[test] + fn v0_parses_encryption_and_decryption_keys_from_distinct_properties() { + let mut map: BTreeMap = BTreeMap::new(); + map.insert( + REQUIRES_IDENTITY_ENCRYPTION_BOUNDED_KEY.to_string(), + Value::U8(StorageKeyRequirements::Unique as u8), + ); + map.insert( + REQUIRES_IDENTITY_DECRYPTION_BOUNDED_KEY.to_string(), + Value::U8(StorageKeyRequirements::Multiple as u8), + ); + + let config = DataContractConfigV0::get_contract_configuration_properties_v0(&map) + .expect("should parse V0 config"); + + assert_eq!( + config.requires_identity_encryption_bounded_key, + Some(StorageKeyRequirements::Unique) + ); + assert_eq!( + config.requires_identity_decryption_bounded_key, + Some(StorageKeyRequirements::Multiple) + ); + } + + #[test] + fn v0_leaves_bounded_key_fields_none_when_absent() { + let map: BTreeMap = BTreeMap::new(); + let config = DataContractConfigV0::get_contract_configuration_properties_v0(&map) + .expect("should parse V0 config with defaults"); + assert!(config.requires_identity_encryption_bounded_key.is_none()); + assert!(config.requires_identity_decryption_bounded_key.is_none()); + } + + #[test] + fn v0_parses_only_decryption_when_encryption_absent() { + let mut map: BTreeMap = BTreeMap::new(); + map.insert( + REQUIRES_IDENTITY_DECRYPTION_BOUNDED_KEY.to_string(), + Value::U8(StorageKeyRequirements::MultipleReferenceToLatest as u8), + ); + + let config = DataContractConfigV0::get_contract_configuration_properties_v0(&map) + .expect("should parse V0 config"); + + assert!(config.requires_identity_encryption_bounded_key.is_none()); + assert_eq!( + config.requires_identity_decryption_bounded_key, + Some(StorageKeyRequirements::MultipleReferenceToLatest) + ); + } + } } diff --git a/packages/rs-dpp/src/data_contract/config/v0/mod.rs b/packages/rs-dpp/src/data_contract/config/v0/mod.rs index f3bd62e0e5e..a29342e90f4 100644 --- a/packages/rs-dpp/src/data_contract/config/v0/mod.rs +++ b/packages/rs-dpp/src/data_contract/config/v0/mod.rs @@ -180,7 +180,7 @@ impl DataContractConfigV0 { .transpose()?; let requires_identity_decryption_bounded_key = contract - .get_optional_integer::(config::property::REQUIRES_IDENTITY_ENCRYPTION_BOUNDED_KEY)? + .get_optional_integer::(config::property::REQUIRES_IDENTITY_DECRYPTION_BOUNDED_KEY)? .map(|int| int.try_into()) .transpose()?; From 13da272d3e062cb78f59aad784548d3b4a99910e Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Wed, 22 Apr 2026 14:14:37 +0800 Subject: [PATCH 2/2] test(dpp): pin V0 config parser consensus-frozen quirk Reverts the functional change from the prior commit on this branch. `DataContractConfigV0::get_contract_configuration_properties_v0` reads the decryption bounded-key field from the ENCRYPTION property key. This is not a safe-to-patch typo: its behavior is part of V0 protocol output and cannot be changed without risking a chain fork on replay. V1's parser is already correct and handles the DECRYPTION key distinctly. - Restore the original V0 lookup. - Add a `CONSENSUS-FROZEN BUG` comment at the site so the quirk is not re-"fixed" by a future refactor. - Replace the earlier regression tests with a lock-in test module that asserts the observable V0 behavior: * encryption property populates BOTH fields, * decryption property is ignored entirely, * encryption wins when both are set, * neither set leaves both `None`. --- .../rs-dpp/src/data_contract/config/mod.rs | 78 ++++++++++++++----- .../rs-dpp/src/data_contract/config/v0/mod.rs | 8 +- 2 files changed, 65 insertions(+), 21 deletions(-) diff --git a/packages/rs-dpp/src/data_contract/config/mod.rs b/packages/rs-dpp/src/data_contract/config/mod.rs index eef81776206..eff57131bb8 100644 --- a/packages/rs-dpp/src/data_contract/config/mod.rs +++ b/packages/rs-dpp/src/data_contract/config/mod.rs @@ -584,7 +584,14 @@ mod tests { } } - mod get_contract_configuration_properties { + /// V0's `get_contract_configuration_properties_v0` has a historical + /// copy-paste quirk: the decryption bounded-key field is parsed from + /// the `requiresIdentityEncryptionBoundedKey` property (not the matching + /// DECRYPTION one). This is part of V0 protocol behavior and MUST NOT be + /// changed — altering it would fork the chain. V1 parses correctly; see + /// `v1/mod.rs`. These tests lock the V0 behavior in place so the quirk + /// is not silently "fixed" by a future well-intentioned refactor. + mod get_contract_configuration_properties_v0_consensus_lock { use super::*; use crate::data_contract::config::property::{ REQUIRES_IDENTITY_DECRYPTION_BOUNDED_KEY, REQUIRES_IDENTITY_ENCRYPTION_BOUNDED_KEY, @@ -592,21 +599,16 @@ mod tests { use platform_value::Value; use std::collections::BTreeMap; - /// Regression test for a copy-paste bug where V0 parsing read both - /// encryption and decryption bounded-key fields from the same map key - /// (`requiresIdentityEncryptionBoundedKey`), making it impossible to - /// set them to different values via the map-based parser. + /// When the ENCRYPTION property is set, V0 applies that value to + /// BOTH the encryption and decryption fields — because the parser + /// reads both from the same key. #[test] - fn v0_parses_encryption_and_decryption_keys_from_distinct_properties() { + fn encryption_property_populates_both_fields() { let mut map: BTreeMap = BTreeMap::new(); map.insert( REQUIRES_IDENTITY_ENCRYPTION_BOUNDED_KEY.to_string(), Value::U8(StorageKeyRequirements::Unique as u8), ); - map.insert( - REQUIRES_IDENTITY_DECRYPTION_BOUNDED_KEY.to_string(), - Value::U8(StorageKeyRequirements::Multiple as u8), - ); let config = DataContractConfigV0::get_contract_configuration_properties_v0(&map) .expect("should parse V0 config"); @@ -617,35 +619,71 @@ mod tests { ); assert_eq!( config.requires_identity_decryption_bounded_key, - Some(StorageKeyRequirements::Multiple) + Some(StorageKeyRequirements::Unique), + "V0 consensus quirk: decryption field is read from the ENCRYPTION key" ); } + /// When ONLY the DECRYPTION property is set, V0 ignores it entirely + /// — neither field is populated, because V0 never reads the + /// DECRYPTION key. #[test] - fn v0_leaves_bounded_key_fields_none_when_absent() { - let map: BTreeMap = BTreeMap::new(); + fn decryption_property_is_ignored_by_v0() { + let mut map: BTreeMap = BTreeMap::new(); + map.insert( + REQUIRES_IDENTITY_DECRYPTION_BOUNDED_KEY.to_string(), + Value::U8(StorageKeyRequirements::MultipleReferenceToLatest as u8), + ); + let config = DataContractConfigV0::get_contract_configuration_properties_v0(&map) - .expect("should parse V0 config with defaults"); - assert!(config.requires_identity_encryption_bounded_key.is_none()); - assert!(config.requires_identity_decryption_bounded_key.is_none()); + .expect("should parse V0 config"); + + assert!( + config.requires_identity_encryption_bounded_key.is_none(), + "V0 does not read the DECRYPTION property at all" + ); + assert!( + config.requires_identity_decryption_bounded_key.is_none(), + "V0 consensus quirk: decryption field is NOT sourced from the DECRYPTION key" + ); } + /// When BOTH properties are set, the ENCRYPTION value wins for both + /// fields; the DECRYPTION property is ignored. #[test] - fn v0_parses_only_decryption_when_encryption_absent() { + fn encryption_wins_when_both_properties_set() { let mut map: BTreeMap = BTreeMap::new(); + map.insert( + REQUIRES_IDENTITY_ENCRYPTION_BOUNDED_KEY.to_string(), + Value::U8(StorageKeyRequirements::Unique as u8), + ); map.insert( REQUIRES_IDENTITY_DECRYPTION_BOUNDED_KEY.to_string(), - Value::U8(StorageKeyRequirements::MultipleReferenceToLatest as u8), + Value::U8(StorageKeyRequirements::Multiple as u8), ); let config = DataContractConfigV0::get_contract_configuration_properties_v0(&map) .expect("should parse V0 config"); - assert!(config.requires_identity_encryption_bounded_key.is_none()); + assert_eq!( + config.requires_identity_encryption_bounded_key, + Some(StorageKeyRequirements::Unique) + ); assert_eq!( config.requires_identity_decryption_bounded_key, - Some(StorageKeyRequirements::MultipleReferenceToLatest) + Some(StorageKeyRequirements::Unique), + "V0 consensus quirk: the DECRYPTION property is ignored" ); } + + /// Sanity check: with neither property set, both fields stay `None`. + #[test] + fn neither_property_set_leaves_both_none() { + let map: BTreeMap = BTreeMap::new(); + let config = DataContractConfigV0::get_contract_configuration_properties_v0(&map) + .expect("should parse V0 config with defaults"); + assert!(config.requires_identity_encryption_bounded_key.is_none()); + assert!(config.requires_identity_decryption_bounded_key.is_none()); + } } } diff --git a/packages/rs-dpp/src/data_contract/config/v0/mod.rs b/packages/rs-dpp/src/data_contract/config/v0/mod.rs index a29342e90f4..c8e83e2854e 100644 --- a/packages/rs-dpp/src/data_contract/config/v0/mod.rs +++ b/packages/rs-dpp/src/data_contract/config/v0/mod.rs @@ -179,8 +179,14 @@ impl DataContractConfigV0 { .map(|int| int.try_into()) .transpose()?; + // CONSENSUS-FROZEN BUG: this intentionally reads from + // `REQUIRES_IDENTITY_ENCRYPTION_BOUNDED_KEY` (not the matching + // DECRYPTION constant). The V0 parser shipped this way and its output + // is part of V0 protocol behavior, so it must not be changed even + // though it looks like a copy-paste typo. V1 reads from the correct + // DECRYPTION key — see v1/mod.rs. Do not "fix" this line. let requires_identity_decryption_bounded_key = contract - .get_optional_integer::(config::property::REQUIRES_IDENTITY_DECRYPTION_BOUNDED_KEY)? + .get_optional_integer::(config::property::REQUIRES_IDENTITY_ENCRYPTION_BOUNDED_KEY)? .map(|int| int.try_into()) .transpose()?;