From e2daef17ac70c7c5ce0bb39a8b058919a1fbfe32 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Wed, 20 May 2026 19:13:04 +0700 Subject: [PATCH 1/5] fix(drive-abci): correct DECRYPTION branch in SingleContractDocumentType bounds validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `validate_identity_public_key_contract_bounds` v0 consulted `document_type.requires_identity_encryption_bounded_key()` in the `Purpose::DECRYPTION` arm of `ContractBounds::SingleContractDocumentType`. That branch should consult `requires_identity_decryption_bounded_key()` — asymmetric document-type configs were either wrongly rejected or wrongly accepted depending on which side was set. v1 is a structural copy of v0 with that single call corrected, gated by `validate_identity_public_key_contract_bounds: 1` inside the existing `DRIVE_ABCI_VALIDATION_VERSIONS_V8` (which is referenced only by `PLATFORM_V12`, the in-development 3.1.0 release). Pre-v12 chain replay continues to route through v0 verbatim. Adds a differential test pinning both the legacy v0 behavior and the v1 fix on a doc type that sets `requiresIdentityDecryptionBoundedKey` but not `requiresIdentityEncryptionBoundedKey` — the missing coverage hole that originally let this slip through. Supersedes #2996 (Paul DeLucia's original fix, which couldn't be merged because its v12/v8 scaffolding conflicted with #3017). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../mod.rs | 174 +++++++++- .../v1/mod.rs | 308 ++++++++++++++++++ .../drive_abci_validation_versions/v8.rs | 2 +- 3 files changed, 482 insertions(+), 2 deletions(-) create mode 100644 packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/v1/mod.rs diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/mod.rs index aabfec69beb..aeab196a55d 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/mod.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/mod.rs @@ -8,8 +8,10 @@ use crate::error::Error; use crate::error::execution::ExecutionError; use crate::execution::types::state_transition_execution_context::StateTransitionExecutionContext; use crate::execution::validation::state_transition::common::validate_identity_public_key_contract_bounds::v0::validate_identity_public_keys_contract_bounds_v0; +use crate::execution::validation::state_transition::common::validate_identity_public_key_contract_bounds::v1::validate_identity_public_keys_contract_bounds_v1; pub mod v0; +pub mod v1; pub(crate) fn validate_identity_public_keys_contract_bounds( identity_id: Identifier, @@ -34,10 +36,180 @@ pub(crate) fn validate_identity_public_keys_contract_bounds( execution_context, platform_version, ), + 1 => validate_identity_public_keys_contract_bounds_v1( + identity_id, + identity_public_keys_with_witness, + drive, + transaction, + execution_context, + platform_version, + ), version => Err(Error::Execution(ExecutionError::UnknownVersionMismatch { method: "validate_identity_public_keys_contract_bounds".to_string(), - known_versions: vec![0], + known_versions: vec![0, 1], received: version, })), } } + +#[cfg(test)] +mod tests { + //! Differential test pinning the v0 vs v1 behavior on the previously-buggy branch: + //! `Purpose::DECRYPTION` against `ContractBounds::SingleContractDocumentType`, where + //! the document type sets `requiresIdentityDecryptionBoundedKey` but not + //! `requiresIdentityEncryptionBoundedKey`. v0 mistakenly checked the encryption + //! requirement; v1 correctly checks the decryption requirement. + //! + //! v0's behavior is frozen for chain replay — the v0 assertion below pins it. + use super::v0::validate_identity_public_keys_contract_bounds_v0; + use super::v1::validate_identity_public_keys_contract_bounds_v1; + use crate::execution::types::state_transition_execution_context::StateTransitionExecutionContext; + use crate::test::helpers::setup::TestPlatformBuilder; + use dpp::block::block_info::BlockInfo; + use dpp::consensus::basic::BasicError; + use dpp::consensus::ConsensusError; + use dpp::data_contract::accessors::v0::DataContractV0Getters; + use dpp::data_contract::DataContractFactory; + use dpp::identifier::Identifier; + use dpp::identity::contract_bounds::ContractBounds; + use dpp::identity::{KeyType, Purpose, SecurityLevel}; + use dpp::platform_value::{platform_value, BinaryData}; + use dpp::state_transition::public_key_in_creation::v0::IdentityPublicKeyInCreationV0; + use dpp::state_transition::public_key_in_creation::IdentityPublicKeyInCreation; + use dpp::version::{DefaultForPlatformVersion, PlatformVersion}; + + /// Document type with decryption bounds set and encryption bounds unset — exactly the + /// asymmetric case the v0 bug mishandled. + fn build_contract_with_decryption_only_bounds( + platform_version: &PlatformVersion, + ) -> dpp::data_contract::DataContract { + let factory = DataContractFactory::new(platform_version.protocol_version).expect("factory"); + let schemas = platform_value!({ + "note": { + "type": "object", + "requiresIdentityDecryptionBoundedKey": 0_u64, + "properties": { + "message": {"type": "string", "position": 0, "maxLength": 100_u32}, + }, + "additionalProperties": false, + } + }); + factory + .create_with_value_config(Identifier::random(), 0, schemas, None, None) + .expect("contract built") + .data_contract_owned() + } + + fn make_decryption_key_bound_to_doc_type( + contract_id: Identifier, + document_type_name: String, + ) -> IdentityPublicKeyInCreation { + IdentityPublicKeyInCreationV0 { + id: 0, + key_type: KeyType::ECDSA_SECP256K1, + purpose: Purpose::DECRYPTION, + security_level: SecurityLevel::HIGH, + contract_bounds: Some(ContractBounds::SingleContractDocumentType { + id: contract_id, + document_type_name, + }), + read_only: false, + data: BinaryData::new(vec![0u8; 33]), + signature: BinaryData::default(), + } + .into() + } + + #[test] + fn v0_wrongly_rejects_decryption_key_when_only_decryption_bounds_set() { + let platform_version = PlatformVersion::latest(); + let platform = TestPlatformBuilder::new() + .build_with_mock_rpc() + .set_genesis_state(); + + let contract = build_contract_with_decryption_only_bounds(platform_version); + let contract_id = contract.id(); + platform + .drive + .apply_contract( + &contract, + BlockInfo::default(), + true, + None, + None, + platform_version, + ) + .expect("contract applied"); + + let key = make_decryption_key_bound_to_doc_type(contract_id, "note".to_string()); + let mut execution_context = + StateTransitionExecutionContext::default_for_platform_version(platform_version) + .expect("execution context"); + + let result = validate_identity_public_keys_contract_bounds_v0( + Identifier::random(), + &[key], + &platform.drive, + None, + &mut execution_context, + platform_version, + ) + .expect("v0 returns Ok"); + + assert!( + !result.is_valid(), + "v0 has the bug — it consults encryption bounds for a DECRYPTION key" + ); + match &result.errors[0] { + ConsensusError::BasicError(BasicError::DataContractBoundsNotPresentError(_)) => {} + other => panic!( + "expected v0 to wrongly emit DataContractBoundsNotPresentError, got {:?}", + other + ), + } + } + + #[test] + fn v1_accepts_decryption_key_when_decryption_bounds_present() { + let platform_version = PlatformVersion::latest(); + let platform = TestPlatformBuilder::new() + .build_with_mock_rpc() + .set_genesis_state(); + + let contract = build_contract_with_decryption_only_bounds(platform_version); + let contract_id = contract.id(); + platform + .drive + .apply_contract( + &contract, + BlockInfo::default(), + true, + None, + None, + platform_version, + ) + .expect("contract applied"); + + let key = make_decryption_key_bound_to_doc_type(contract_id, "note".to_string()); + let mut execution_context = + StateTransitionExecutionContext::default_for_platform_version(platform_version) + .expect("execution context"); + + let result = validate_identity_public_keys_contract_bounds_v1( + Identifier::random(), + &[key], + &platform.drive, + None, + &mut execution_context, + platform_version, + ) + .expect("v1 returns Ok"); + + assert!( + result.is_valid(), + "v1 fix: a DECRYPTION key targeting a doc type with decryption bounds is valid; \ + got errors: {:?}", + result.errors + ); + } +} diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/v1/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/v1/mod.rs new file mode 100644 index 00000000000..c33684f49f8 --- /dev/null +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/v1/mod.rs @@ -0,0 +1,308 @@ +//! v1 fixes the DECRYPTION branch of `SingleContractDocumentType` bounds validation: +//! v0 mistakenly called `requires_identity_encryption_bounded_key()` there. The rest of +//! the file is intentionally kept structurally identical to v0 to keep the diff auditable. + +use crate::error::Error; +use crate::execution::types::state_transition_execution_context::StateTransitionExecutionContext; +use dpp::consensus::basic::document::{ + DataContractNotPresentError, InvalidDocumentTypeError, +}; +use dpp::consensus::basic::identity::{DataContractBoundsNotPresentError, InvalidKeyPurposeForContractBoundsError}; +use dpp::consensus::basic::BasicError; +use dpp::consensus::ConsensusError; +use dpp::consensus::state::identity::identity_public_key_already_exists_for_unique_contract_bounds_error::IdentityPublicKeyAlreadyExistsForUniqueContractBoundsError; +use dpp::consensus::state::state_error::StateError; +use dpp::data_contract::accessors::v0::DataContractV0Getters; +use dpp::data_contract::config::v0::DataContractConfigGettersV0; +use dpp::data_contract::document_type::accessors::DocumentTypeV0Getters; +use dpp::data_contract::storage_requirements::keys_for_document_type::StorageKeyRequirements; +use dpp::identifier::Identifier; +use dpp::identity::contract_bounds::ContractBounds; +use dpp::identity::identity_public_key::accessors::v0::IdentityPublicKeyGettersV0; +use dpp::identity::Purpose::{DECRYPTION, ENCRYPTION}; +use dpp::state_transition::public_key_in_creation::accessors::IdentityPublicKeyInCreationV0Getters; +use dpp::state_transition::public_key_in_creation::IdentityPublicKeyInCreation; +use dpp::validation::SimpleConsensusValidationResult; +use dpp::version::PlatformVersion; +use drive::drive::Drive; +use drive::drive::identity::key::fetch::{IdentityKeysRequest, KeyKindRequestType, KeyRequestType, OptionalSingleIdentityPublicKeyOutcome}; +use drive::grovedb::TransactionArg; + +pub(super) fn validate_identity_public_keys_contract_bounds_v1( + identity_id: Identifier, + identity_public_keys_with_witness: &[IdentityPublicKeyInCreation], + drive: &Drive, + transaction: TransactionArg, + execution_context: &mut StateTransitionExecutionContext, + platform_version: &PlatformVersion, +) -> Result { + let consensus_validation_results = identity_public_keys_with_witness + .iter() + .map(|identity_public_key| { + validate_identity_public_key_contract_bounds_v1( + identity_id, + identity_public_key, + drive, + transaction, + execution_context, + platform_version, + ) + }) + .collect::, Error>>()?; + Ok(SimpleConsensusValidationResult::merge_many_errors( + consensus_validation_results, + )) +} + +fn validate_identity_public_key_contract_bounds_v1( + identity_id: Identifier, + identity_public_key_in_creation: &IdentityPublicKeyInCreation, + drive: &Drive, + transaction: TransactionArg, + _execution_context: &mut StateTransitionExecutionContext, + platform_version: &PlatformVersion, +) -> Result { + //todo: we should add to the execution context the cost of fetching contracts + let purpose = identity_public_key_in_creation.purpose(); + if let Some(contract_bounds) = identity_public_key_in_creation.contract_bounds() { + match contract_bounds { + ContractBounds::SingleContract { id: contract_id } => { + // we should fetch the contract + let contract = drive.get_contract_with_fetch_info( + contract_id.to_buffer(), + false, + transaction, + platform_version, + )?; + match contract { + None => Ok(SimpleConsensusValidationResult::new_with_error( + ConsensusError::BasicError(BasicError::DataContractNotPresentError( + DataContractNotPresentError::new(*contract_id), + )), + )), + Some(contract) => { + match purpose { + ENCRYPTION => { + let Some(requirements) = contract + .contract + .config() + .requires_identity_encryption_bounded_key() + else { + return Ok(SimpleConsensusValidationResult::new_with_error( + ConsensusError::BasicError( + BasicError::DataContractBoundsNotPresentError( + DataContractBoundsNotPresentError::new( + *contract_id, + ), + ), + ), + )); + }; + + match requirements { + // We should make sure no other key exists for these bounds + StorageKeyRequirements::Unique => { + let key_request = IdentityKeysRequest { + identity_id: identity_id.to_buffer(), + request_type: KeyRequestType::ContractBoundKey( + contract_id.to_buffer(), + purpose, + KeyKindRequestType::CurrentKeyOfKindRequest, + ), + limit: None, + offset: None, + }; + let maybe_conflicting_key = drive.fetch_identity_keys::(key_request, transaction, platform_version)?; + if let Some(conflicting_key) = maybe_conflicting_key { + Ok(SimpleConsensusValidationResult::new_with_error(ConsensusError::StateError(StateError::IdentityPublicKeyAlreadyExistsForUniqueContractBoundsError(IdentityPublicKeyAlreadyExistsForUniqueContractBoundsError::new(identity_id, *contract_id, purpose, identity_public_key_in_creation.id(), conflicting_key.id()))))) + } else { + Ok(SimpleConsensusValidationResult::new()) + } + } + StorageKeyRequirements::Multiple + | StorageKeyRequirements::MultipleReferenceToLatest => { + Ok(SimpleConsensusValidationResult::new()) + } + } + } + DECRYPTION => { + let Some(requirements) = contract + .contract + .config() + .requires_identity_decryption_bounded_key() + else { + return Ok(SimpleConsensusValidationResult::new_with_error( + ConsensusError::BasicError( + BasicError::DataContractBoundsNotPresentError( + DataContractBoundsNotPresentError::new( + *contract_id, + ), + ), + ), + )); + }; + + match requirements { + StorageKeyRequirements::Unique => { + // We should make sure no other key exists for these bounds + let key_request = IdentityKeysRequest { + identity_id: identity_id.to_buffer(), + request_type: KeyRequestType::ContractBoundKey( + contract_id.to_buffer(), + purpose, + KeyKindRequestType::CurrentKeyOfKindRequest, + ), + limit: None, + offset: None, + }; + let maybe_conflicting_key = drive.fetch_identity_keys::(key_request, transaction, platform_version)?; + if let Some(conflicting_key) = maybe_conflicting_key { + Ok(SimpleConsensusValidationResult::new_with_error(ConsensusError::StateError(StateError::IdentityPublicKeyAlreadyExistsForUniqueContractBoundsError(IdentityPublicKeyAlreadyExistsForUniqueContractBoundsError::new(identity_id, *contract_id, purpose, identity_public_key_in_creation.id(), conflicting_key.id()))))) + } else { + Ok(SimpleConsensusValidationResult::new()) + } + } + StorageKeyRequirements::Multiple + | StorageKeyRequirements::MultipleReferenceToLatest => { + Ok(SimpleConsensusValidationResult::new()) + } + } + } + purpose => Ok(SimpleConsensusValidationResult::new_with_error( + ConsensusError::BasicError( + BasicError::InvalidKeyPurposeForContractBoundsError( + InvalidKeyPurposeForContractBoundsError::new( + purpose, + vec![ENCRYPTION, DECRYPTION], + ), + ), + ), + )), + } + } + } + } + ContractBounds::SingleContractDocumentType { + id: contract_id, + document_type_name, + } => { + let contract = drive.get_contract_with_fetch_info( + contract_id.to_buffer(), + false, + transaction, + platform_version, + )?; + match contract { + None => Ok(SimpleConsensusValidationResult::new_with_error( + ConsensusError::BasicError(BasicError::DataContractNotPresentError( + DataContractNotPresentError::new(*contract_id), + )), + )), + Some(contract) => { + let document_type = contract + .contract + .document_type_optional_for_name(document_type_name.as_str()); + match document_type { + None => Ok(SimpleConsensusValidationResult::new_with_error( + ConsensusError::BasicError(BasicError::InvalidDocumentTypeError( + InvalidDocumentTypeError::new( + document_type_name.clone(), + *contract_id, + ), + )), + )), + Some(document_type) => { + match purpose { + ENCRYPTION => { + let Some(requirements) = document_type + .requires_identity_encryption_bounded_key() + else { + return Ok(SimpleConsensusValidationResult::new_with_error( + ConsensusError::BasicError( + BasicError::DataContractBoundsNotPresentError( + DataContractBoundsNotPresentError::new(*contract_id), + ), + ), + )); + }; + + match requirements { + StorageKeyRequirements::Unique => { + // We should make sure no other key exists for these bounds + let key_request = IdentityKeysRequest { + identity_id: identity_id.to_buffer(), + request_type: KeyRequestType::ContractDocumentTypeBoundKey(contract_id.to_buffer(), document_type_name.clone(), purpose, KeyKindRequestType::CurrentKeyOfKindRequest), + limit: None, + offset: None, + }; + let maybe_conflicting_key = drive.fetch_identity_keys::(key_request, transaction, platform_version)?; + if let Some(conflicting_key) = maybe_conflicting_key + { + Ok(SimpleConsensusValidationResult::new_with_error(ConsensusError::StateError(StateError::IdentityPublicKeyAlreadyExistsForUniqueContractBoundsError(IdentityPublicKeyAlreadyExistsForUniqueContractBoundsError::new(identity_id, *contract_id, purpose, identity_public_key_in_creation.id(), conflicting_key.id()))))) + } else { + Ok(SimpleConsensusValidationResult::new()) + } + } + StorageKeyRequirements::Multiple + | StorageKeyRequirements::MultipleReferenceToLatest => { + Ok(SimpleConsensusValidationResult::new()) + } + } + } + DECRYPTION => { + // v1 fix: v0 called requires_identity_encryption_bounded_key() here. + let Some(requirements) = document_type + .requires_identity_decryption_bounded_key() + else { + return Ok(SimpleConsensusValidationResult::new_with_error( + ConsensusError::BasicError( + BasicError::DataContractBoundsNotPresentError( + DataContractBoundsNotPresentError::new(*contract_id), + ), + ), + )); + }; + + match requirements { + StorageKeyRequirements::Unique => { + let key_request = IdentityKeysRequest { + identity_id: identity_id.to_buffer(), + request_type: KeyRequestType::ContractDocumentTypeBoundKey(contract_id.to_buffer(), document_type_name.clone(), purpose, KeyKindRequestType::CurrentKeyOfKindRequest), + limit: None, + offset: None, + }; + let maybe_conflicting_key = drive.fetch_identity_keys::(key_request, transaction, platform_version)?; + if let Some(conflicting_key) = maybe_conflicting_key + { + Ok(SimpleConsensusValidationResult::new_with_error(ConsensusError::StateError(StateError::IdentityPublicKeyAlreadyExistsForUniqueContractBoundsError(IdentityPublicKeyAlreadyExistsForUniqueContractBoundsError::new(identity_id, *contract_id, purpose, identity_public_key_in_creation.id(), conflicting_key.id()))))) + } else { + Ok(SimpleConsensusValidationResult::new()) + } + } + StorageKeyRequirements::Multiple + | StorageKeyRequirements::MultipleReferenceToLatest => { + Ok(SimpleConsensusValidationResult::new()) + } + } + } + _ => Ok(SimpleConsensusValidationResult::new_with_error( + ConsensusError::BasicError( + BasicError::InvalidKeyPurposeForContractBoundsError( + InvalidKeyPurposeForContractBoundsError::new( + purpose, + vec![ENCRYPTION, DECRYPTION], + ), + ), + ), + )), + } + } + } + } + } + } + } + } else { + Ok(SimpleConsensusValidationResult::new()) + } +} diff --git a/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v8.rs b/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v8.rs index f7e83c01ee8..a8689ef7c45 100644 --- a/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v8.rs +++ b/packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v8.rs @@ -27,7 +27,7 @@ pub const DRIVE_ABCI_VALIDATION_VERSIONS_V8: DriveAbciValidationVersions = fetch_asset_lock_transaction_output_sync: 0, verify_asset_lock_is_not_spent_and_has_enough_balance: 0, }, - validate_identity_public_key_contract_bounds: 0, + validate_identity_public_key_contract_bounds: 1, validate_identity_public_key_ids_dont_exist_in_state: 0, validate_identity_public_key_ids_exist_in_state: 0, validate_state_transition_identity_signed: 0, From ea84f9b9b5e9cfa9ba622c810ecf0997cb420c2d Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Wed, 20 May 2026 19:51:23 +0700 Subject: [PATCH 2/5] fix(drive-abci): bill contract + identity-key reads in bounds validation (audit N6/N7) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `validate_identity_public_key_contract_bounds` had an explicit `//todo: we should add to the execution context the cost of fetching contracts` on v0 and discarded every grovedb read (contract fetches + unique-key lookups). Under paid-error semantics, an identity_update that failed bounds validation under-charged the user for the validation work that ran. Audit entries N6 / N7 in `docs/paid-error-fee-audit.md` (PR #3670), tracked by issue #3673. Folded into v1 of the bounds validator (already gated at PROTOCOL_VERSION_12 via DRIVE_ABCI_VALIDATION_VERSIONS_V8 by the previous commit). v0 stays byte-identical for chain replay. - `rs-drive`: new `pub fn fetch_identity_keys_with_costs` (mirrors the existing `fetch_identity_balance_with_costs` precedent — takes `&Epoch`, returns `(T, FeeResult)`). - `rs-drive-abci` v1: switched contract fetches to `get_contract_with_fetch_info_and_fee` and identity-key lookups to the new `fetch_identity_keys_with_costs`, pushing each returned `FeeResult` into `execution_context` via `ValidationOperation::PrecalculatedOperation`. Also refactored the body to factor out the unique-key check into a single helper (was duplicated across four bounds × purpose combinations). - Dispatcher now takes `epoch: &Epoch`. v0 ignores it (preserved). - `identity_update::validate_state_v0` passes `platform.state.last_committed_block_epoch_ref()` to the dispatcher. New test `v1_bills_contract_fetch_and_unique_key_lookup` asserts the execution context receives at least two `PrecalculatedOperation` entries with non-zero processing fees after v1 runs. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../mod.rs | 116 ++++- .../v1/mod.rs | 467 ++++++++---------- .../identity_update/state/v0/mod.rs | 2 + .../key/fetch/fetch_identity_keys/mod.rs | 34 ++ .../key/fetch/fetch_identity_keys/v0/mod.rs | 29 ++ 5 files changed, 378 insertions(+), 270 deletions(-) diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/mod.rs index aeab196a55d..7f981da7c65 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/mod.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/mod.rs @@ -1,22 +1,29 @@ +use crate::error::execution::ExecutionError; +use crate::error::Error; +use crate::execution::types::state_transition_execution_context::StateTransitionExecutionContext; +use crate::execution::validation::state_transition::common::validate_identity_public_key_contract_bounds::v0::validate_identity_public_keys_contract_bounds_v0; +use crate::execution::validation::state_transition::common::validate_identity_public_key_contract_bounds::v1::validate_identity_public_keys_contract_bounds_v1; +use dpp::block::epoch::Epoch; use dpp::identifier::Identifier; use dpp::state_transition::public_key_in_creation::IdentityPublicKeyInCreation; use dpp::validation::SimpleConsensusValidationResult; +use dpp::version::PlatformVersion; use drive::drive::Drive; use drive::grovedb::TransactionArg; -use dpp::version::PlatformVersion; -use crate::error::Error; -use crate::error::execution::ExecutionError; -use crate::execution::types::state_transition_execution_context::StateTransitionExecutionContext; -use crate::execution::validation::state_transition::common::validate_identity_public_key_contract_bounds::v0::validate_identity_public_keys_contract_bounds_v0; -use crate::execution::validation::state_transition::common::validate_identity_public_key_contract_bounds::v1::validate_identity_public_keys_contract_bounds_v1; pub mod v0; pub mod v1; +/// Validates the contract bounds attached to each public key in `identity_public_keys_with_witness`. +/// +/// `epoch` is used by v1+ to bill the underlying grovedb reads to `execution_context`; v0 +/// ignores it (v0 didn't bill these reads — pre-PROTOCOL_VERSION_12 behavior is preserved +/// verbatim for chain replay). pub(crate) fn validate_identity_public_keys_contract_bounds( identity_id: Identifier, identity_public_keys_with_witness: &[IdentityPublicKeyInCreation], drive: &Drive, + epoch: &Epoch, transaction: TransactionArg, execution_context: &mut StateTransitionExecutionContext, platform_version: &PlatformVersion, @@ -28,18 +35,22 @@ pub(crate) fn validate_identity_public_keys_contract_bounds( .common_validation_methods .validate_identity_public_key_contract_bounds { - 0 => validate_identity_public_keys_contract_bounds_v0( - identity_id, - identity_public_keys_with_witness, - drive, - transaction, - execution_context, - platform_version, - ), + 0 => { + let _ = epoch; // v0 doesn't bill these reads — by design, for chain replay. + validate_identity_public_keys_contract_bounds_v0( + identity_id, + identity_public_keys_with_witness, + drive, + transaction, + execution_context, + platform_version, + ) + } 1 => validate_identity_public_keys_contract_bounds_v1( identity_id, identity_public_keys_with_witness, drive, + epoch, transaction, execution_context, platform_version, @@ -63,9 +74,13 @@ mod tests { //! v0's behavior is frozen for chain replay — the v0 assertion below pins it. use super::v0::validate_identity_public_keys_contract_bounds_v0; use super::v1::validate_identity_public_keys_contract_bounds_v1; - use crate::execution::types::state_transition_execution_context::StateTransitionExecutionContext; + use crate::execution::types::execution_operation::ValidationOperation; + use crate::execution::types::state_transition_execution_context::{ + StateTransitionExecutionContext, StateTransitionExecutionContextMethodsV0, + }; use crate::test::helpers::setup::TestPlatformBuilder; use dpp::block::block_info::BlockInfo; + use dpp::block::epoch::Epoch; use dpp::consensus::basic::BasicError; use dpp::consensus::ConsensusError; use dpp::data_contract::accessors::v0::DataContractV0Getters; @@ -195,10 +210,12 @@ mod tests { StateTransitionExecutionContext::default_for_platform_version(platform_version) .expect("execution context"); + let epoch = Epoch::new(0).expect("epoch 0"); let result = validate_identity_public_keys_contract_bounds_v1( Identifier::random(), &[key], &platform.drive, + &epoch, None, &mut execution_context, platform_version, @@ -212,4 +229,73 @@ mod tests { result.errors ); } + + #[test] + fn v1_bills_contract_fetch_and_unique_key_lookup() { + // v0 dropped these grovedb-read costs on the floor (audit N6/N7). v1 must push them + // into the execution context so paid-error / successful-action billing sees them. + let platform_version = PlatformVersion::latest(); + let platform = TestPlatformBuilder::new() + .build_with_mock_rpc() + .set_genesis_state(); + + let contract = build_contract_with_decryption_only_bounds(platform_version); + let contract_id = contract.id(); + platform + .drive + .apply_contract( + &contract, + BlockInfo::default(), + true, + None, + None, + platform_version, + ) + .expect("contract applied"); + + let key = make_decryption_key_bound_to_doc_type(contract_id, "note".to_string()); + let mut execution_context = + StateTransitionExecutionContext::default_for_platform_version(platform_version) + .expect("execution context"); + let epoch = Epoch::new(0).expect("epoch 0"); + + let result = validate_identity_public_keys_contract_bounds_v1( + Identifier::random(), + &[key], + &platform.drive, + &epoch, + None, + &mut execution_context, + platform_version, + ) + .expect("v1 returns Ok"); + assert!( + result.is_valid(), + "expected valid; got: {:?}", + result.errors + ); + + // Should have at least two billing entries: one for the contract fetch and one for the + // uniqueness key lookup. Both must be non-zero processing fees. + let precalculated: Vec<_> = execution_context + .operations_slice() + .iter() + .filter_map(|op| match op { + ValidationOperation::PrecalculatedOperation(fee) => Some(fee), + _ => None, + }) + .collect(); + assert!( + precalculated.len() >= 2, + "expected at least 2 billed grovedb reads, got {}: {:?}", + precalculated.len(), + precalculated + ); + let total_processing: u64 = precalculated.iter().map(|f| f.processing_fee).sum(); + assert!( + total_processing > 0, + "expected non-zero billed processing fee from contract fetch + key lookup, got {:?}", + precalculated + ); + } } diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/v1/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/v1/mod.rs index c33684f49f8..bbcab687776 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/v1/mod.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/v1/mod.rs @@ -1,17 +1,31 @@ -//! v1 fixes the DECRYPTION branch of `SingleContractDocumentType` bounds validation: -//! v0 mistakenly called `requires_identity_encryption_bounded_key()` there. The rest of -//! the file is intentionally kept structurally identical to v0 to keep the diff auditable. +//! v1 of `validate_identity_public_keys_contract_bounds`. +//! +//! Two behavioral changes vs. v0, both gated together at PROTOCOL_VERSION_12: +//! +//! 1. **Correctness fix.** The `Purpose::DECRYPTION` arm of +//! `ContractBounds::SingleContractDocumentType` now consults +//! `requires_identity_decryption_bounded_key()` instead of v0's mistaken +//! `requires_identity_encryption_bounded_key()`. +//! +//! 2. **Fee accounting (audit N6/N7, follow-up to #3670 / issue #3673).** +//! Every contract fetch and identity-key lookup performed during validation +//! is now billed to the passed-in execution context, instead of being +//! discarded as v0 did (which had an explicit `//todo:` for this). use crate::error::Error; -use crate::execution::types::state_transition_execution_context::StateTransitionExecutionContext; -use dpp::consensus::basic::document::{ - DataContractNotPresentError, InvalidDocumentTypeError, +use crate::execution::types::execution_operation::ValidationOperation; +use crate::execution::types::state_transition_execution_context::{ + StateTransitionExecutionContext, StateTransitionExecutionContextMethodsV0, +}; +use dpp::block::epoch::Epoch; +use dpp::consensus::basic::document::{DataContractNotPresentError, InvalidDocumentTypeError}; +use dpp::consensus::basic::identity::{ + DataContractBoundsNotPresentError, InvalidKeyPurposeForContractBoundsError, }; -use dpp::consensus::basic::identity::{DataContractBoundsNotPresentError, InvalidKeyPurposeForContractBoundsError}; use dpp::consensus::basic::BasicError; -use dpp::consensus::ConsensusError; use dpp::consensus::state::identity::identity_public_key_already_exists_for_unique_contract_bounds_error::IdentityPublicKeyAlreadyExistsForUniqueContractBoundsError; use dpp::consensus::state::state_error::StateError; +use dpp::consensus::ConsensusError; use dpp::data_contract::accessors::v0::DataContractV0Getters; use dpp::data_contract::config::v0::DataContractConfigGettersV0; use dpp::data_contract::document_type::accessors::DocumentTypeV0Getters; @@ -19,38 +33,41 @@ use dpp::data_contract::storage_requirements::keys_for_document_type::StorageKey use dpp::identifier::Identifier; use dpp::identity::contract_bounds::ContractBounds; use dpp::identity::identity_public_key::accessors::v0::IdentityPublicKeyGettersV0; +use dpp::identity::Purpose; use dpp::identity::Purpose::{DECRYPTION, ENCRYPTION}; use dpp::state_transition::public_key_in_creation::accessors::IdentityPublicKeyInCreationV0Getters; use dpp::state_transition::public_key_in_creation::IdentityPublicKeyInCreation; use dpp::validation::SimpleConsensusValidationResult; use dpp::version::PlatformVersion; +use drive::drive::identity::key::fetch::{ + IdentityKeysRequest, KeyKindRequestType, KeyRequestType, OptionalSingleIdentityPublicKeyOutcome, +}; use drive::drive::Drive; -use drive::drive::identity::key::fetch::{IdentityKeysRequest, KeyKindRequestType, KeyRequestType, OptionalSingleIdentityPublicKeyOutcome}; use drive::grovedb::TransactionArg; pub(super) fn validate_identity_public_keys_contract_bounds_v1( identity_id: Identifier, identity_public_keys_with_witness: &[IdentityPublicKeyInCreation], drive: &Drive, + epoch: &Epoch, transaction: TransactionArg, execution_context: &mut StateTransitionExecutionContext, platform_version: &PlatformVersion, ) -> Result { - let consensus_validation_results = identity_public_keys_with_witness - .iter() - .map(|identity_public_key| { - validate_identity_public_key_contract_bounds_v1( - identity_id, - identity_public_key, - drive, - transaction, - execution_context, - platform_version, - ) - }) - .collect::, Error>>()?; + let mut per_key_results = Vec::with_capacity(identity_public_keys_with_witness.len()); + for identity_public_key in identity_public_keys_with_witness { + per_key_results.push(validate_identity_public_key_contract_bounds_v1( + identity_id, + identity_public_key, + drive, + epoch, + transaction, + execution_context, + platform_version, + )?); + } Ok(SimpleConsensusValidationResult::merge_many_errors( - consensus_validation_results, + per_key_results, )) } @@ -58,251 +75,191 @@ fn validate_identity_public_key_contract_bounds_v1( identity_id: Identifier, identity_public_key_in_creation: &IdentityPublicKeyInCreation, drive: &Drive, + epoch: &Epoch, transaction: TransactionArg, - _execution_context: &mut StateTransitionExecutionContext, + execution_context: &mut StateTransitionExecutionContext, platform_version: &PlatformVersion, ) -> Result { - //todo: we should add to the execution context the cost of fetching contracts let purpose = identity_public_key_in_creation.purpose(); - if let Some(contract_bounds) = identity_public_key_in_creation.contract_bounds() { - match contract_bounds { - ContractBounds::SingleContract { id: contract_id } => { - // we should fetch the contract - let contract = drive.get_contract_with_fetch_info( - contract_id.to_buffer(), - false, - transaction, - platform_version, - )?; - match contract { - None => Ok(SimpleConsensusValidationResult::new_with_error( - ConsensusError::BasicError(BasicError::DataContractNotPresentError( - DataContractNotPresentError::new(*contract_id), - )), - )), - Some(contract) => { - match purpose { - ENCRYPTION => { - let Some(requirements) = contract - .contract - .config() - .requires_identity_encryption_bounded_key() - else { - return Ok(SimpleConsensusValidationResult::new_with_error( - ConsensusError::BasicError( - BasicError::DataContractBoundsNotPresentError( - DataContractBoundsNotPresentError::new( - *contract_id, - ), - ), - ), - )); - }; + let Some(contract_bounds) = identity_public_key_in_creation.contract_bounds() else { + return Ok(SimpleConsensusValidationResult::new()); + }; - match requirements { - // We should make sure no other key exists for these bounds - StorageKeyRequirements::Unique => { - let key_request = IdentityKeysRequest { - identity_id: identity_id.to_buffer(), - request_type: KeyRequestType::ContractBoundKey( - contract_id.to_buffer(), - purpose, - KeyKindRequestType::CurrentKeyOfKindRequest, - ), - limit: None, - offset: None, - }; - let maybe_conflicting_key = drive.fetch_identity_keys::(key_request, transaction, platform_version)?; - if let Some(conflicting_key) = maybe_conflicting_key { - Ok(SimpleConsensusValidationResult::new_with_error(ConsensusError::StateError(StateError::IdentityPublicKeyAlreadyExistsForUniqueContractBoundsError(IdentityPublicKeyAlreadyExistsForUniqueContractBoundsError::new(identity_id, *contract_id, purpose, identity_public_key_in_creation.id(), conflicting_key.id()))))) - } else { - Ok(SimpleConsensusValidationResult::new()) - } - } - StorageKeyRequirements::Multiple - | StorageKeyRequirements::MultipleReferenceToLatest => { - Ok(SimpleConsensusValidationResult::new()) - } - } - } - DECRYPTION => { - let Some(requirements) = contract - .contract - .config() - .requires_identity_decryption_bounded_key() - else { - return Ok(SimpleConsensusValidationResult::new_with_error( - ConsensusError::BasicError( - BasicError::DataContractBoundsNotPresentError( - DataContractBoundsNotPresentError::new( - *contract_id, - ), - ), - ), - )); - }; + // Fetch (and bill) the bounded contract. + let contract_id = match contract_bounds { + ContractBounds::SingleContract { id } => *id, + ContractBounds::SingleContractDocumentType { id, .. } => *id, + }; + let (contract_fee, contract_fetch_info) = drive.get_contract_with_fetch_info_and_fee( + contract_id.to_buffer(), + Some(epoch), + false, + transaction, + platform_version, + )?; + if let Some(fee) = contract_fee { + execution_context.add_operation(ValidationOperation::PrecalculatedOperation(fee)); + } + let Some(contract_fetch_info) = contract_fetch_info else { + return Ok(SimpleConsensusValidationResult::new_with_error( + ConsensusError::BasicError(BasicError::DataContractNotPresentError( + DataContractNotPresentError::new(contract_id), + )), + )); + }; + let contract = &contract_fetch_info.contract; - match requirements { - StorageKeyRequirements::Unique => { - // We should make sure no other key exists for these bounds - let key_request = IdentityKeysRequest { - identity_id: identity_id.to_buffer(), - request_type: KeyRequestType::ContractBoundKey( - contract_id.to_buffer(), - purpose, - KeyKindRequestType::CurrentKeyOfKindRequest, - ), - limit: None, - offset: None, - }; - let maybe_conflicting_key = drive.fetch_identity_keys::(key_request, transaction, platform_version)?; - if let Some(conflicting_key) = maybe_conflicting_key { - Ok(SimpleConsensusValidationResult::new_with_error(ConsensusError::StateError(StateError::IdentityPublicKeyAlreadyExistsForUniqueContractBoundsError(IdentityPublicKeyAlreadyExistsForUniqueContractBoundsError::new(identity_id, *contract_id, purpose, identity_public_key_in_creation.id(), conflicting_key.id()))))) - } else { - Ok(SimpleConsensusValidationResult::new()) - } - } - StorageKeyRequirements::Multiple - | StorageKeyRequirements::MultipleReferenceToLatest => { - Ok(SimpleConsensusValidationResult::new()) - } - } - } - purpose => Ok(SimpleConsensusValidationResult::new_with_error( - ConsensusError::BasicError( - BasicError::InvalidKeyPurposeForContractBoundsError( - InvalidKeyPurposeForContractBoundsError::new( - purpose, - vec![ENCRYPTION, DECRYPTION], - ), - ), + match contract_bounds { + ContractBounds::SingleContract { .. } => { + let requirements_for_purpose = match purpose { + ENCRYPTION => contract.config().requires_identity_encryption_bounded_key(), + DECRYPTION => contract.config().requires_identity_decryption_bounded_key(), + purpose => { + return Ok(SimpleConsensusValidationResult::new_with_error( + ConsensusError::BasicError( + BasicError::InvalidKeyPurposeForContractBoundsError( + InvalidKeyPurposeForContractBoundsError::new( + purpose, + vec![ENCRYPTION, DECRYPTION], ), - )), - } - } + ), + ), + )); } - } - ContractBounds::SingleContractDocumentType { - id: contract_id, - document_type_name, - } => { - let contract = drive.get_contract_with_fetch_info( + }; + let Some(requirements) = requirements_for_purpose else { + return Ok(SimpleConsensusValidationResult::new_with_error( + ConsensusError::BasicError(BasicError::DataContractBoundsNotPresentError( + DataContractBoundsNotPresentError::new(contract_id), + )), + )); + }; + check_unique_bound_key( + identity_id, + identity_public_key_in_creation, + contract_id, + purpose, + requirements, + KeyRequestType::ContractBoundKey( + contract_id.to_buffer(), + purpose, + KeyKindRequestType::CurrentKeyOfKindRequest, + ), + drive, + epoch, + transaction, + execution_context, + platform_version, + ) + } + ContractBounds::SingleContractDocumentType { + document_type_name, .. + } => { + let Some(document_type) = + contract.document_type_optional_for_name(document_type_name.as_str()) + else { + return Ok(SimpleConsensusValidationResult::new_with_error( + ConsensusError::BasicError(BasicError::InvalidDocumentTypeError( + InvalidDocumentTypeError::new(document_type_name.clone(), contract_id), + )), + )); + }; + let requirements_for_purpose = match purpose { + ENCRYPTION => document_type.requires_identity_encryption_bounded_key(), + // v1 fix: v0 mistakenly called `requires_identity_encryption_bounded_key()` here. + DECRYPTION => document_type.requires_identity_decryption_bounded_key(), + purpose => { + return Ok(SimpleConsensusValidationResult::new_with_error( + ConsensusError::BasicError( + BasicError::InvalidKeyPurposeForContractBoundsError( + InvalidKeyPurposeForContractBoundsError::new( + purpose, + vec![ENCRYPTION, DECRYPTION], + ), + ), + ), + )); + } + }; + let Some(requirements) = requirements_for_purpose else { + return Ok(SimpleConsensusValidationResult::new_with_error( + ConsensusError::BasicError(BasicError::DataContractBoundsNotPresentError( + DataContractBoundsNotPresentError::new(contract_id), + )), + )); + }; + check_unique_bound_key( + identity_id, + identity_public_key_in_creation, + contract_id, + purpose, + requirements, + KeyRequestType::ContractDocumentTypeBoundKey( contract_id.to_buffer(), - false, + document_type_name.clone(), + purpose, + KeyKindRequestType::CurrentKeyOfKindRequest, + ), + drive, + epoch, + transaction, + execution_context, + platform_version, + ) + } + } +} + +#[allow(clippy::too_many_arguments)] +fn check_unique_bound_key( + identity_id: Identifier, + identity_public_key_in_creation: &IdentityPublicKeyInCreation, + contract_id: Identifier, + purpose: Purpose, + requirements: StorageKeyRequirements, + request_type: KeyRequestType, + drive: &Drive, + epoch: &Epoch, + transaction: TransactionArg, + execution_context: &mut StateTransitionExecutionContext, + platform_version: &PlatformVersion, +) -> Result { + match requirements { + StorageKeyRequirements::Unique => { + let key_request = IdentityKeysRequest { + identity_id: identity_id.to_buffer(), + request_type, + limit: None, + offset: None, + }; + let (maybe_conflicting_key, fee) = drive + .fetch_identity_keys_with_costs::( + key_request, + epoch, transaction, platform_version, )?; - match contract { - None => Ok(SimpleConsensusValidationResult::new_with_error( - ConsensusError::BasicError(BasicError::DataContractNotPresentError( - DataContractNotPresentError::new(*contract_id), - )), - )), - Some(contract) => { - let document_type = contract - .contract - .document_type_optional_for_name(document_type_name.as_str()); - match document_type { - None => Ok(SimpleConsensusValidationResult::new_with_error( - ConsensusError::BasicError(BasicError::InvalidDocumentTypeError( - InvalidDocumentTypeError::new( - document_type_name.clone(), - *contract_id, - ), - )), - )), - Some(document_type) => { - match purpose { - ENCRYPTION => { - let Some(requirements) = document_type - .requires_identity_encryption_bounded_key() - else { - return Ok(SimpleConsensusValidationResult::new_with_error( - ConsensusError::BasicError( - BasicError::DataContractBoundsNotPresentError( - DataContractBoundsNotPresentError::new(*contract_id), - ), - ), - )); - }; - - match requirements { - StorageKeyRequirements::Unique => { - // We should make sure no other key exists for these bounds - let key_request = IdentityKeysRequest { - identity_id: identity_id.to_buffer(), - request_type: KeyRequestType::ContractDocumentTypeBoundKey(contract_id.to_buffer(), document_type_name.clone(), purpose, KeyKindRequestType::CurrentKeyOfKindRequest), - limit: None, - offset: None, - }; - let maybe_conflicting_key = drive.fetch_identity_keys::(key_request, transaction, platform_version)?; - if let Some(conflicting_key) = maybe_conflicting_key - { - Ok(SimpleConsensusValidationResult::new_with_error(ConsensusError::StateError(StateError::IdentityPublicKeyAlreadyExistsForUniqueContractBoundsError(IdentityPublicKeyAlreadyExistsForUniqueContractBoundsError::new(identity_id, *contract_id, purpose, identity_public_key_in_creation.id(), conflicting_key.id()))))) - } else { - Ok(SimpleConsensusValidationResult::new()) - } - } - StorageKeyRequirements::Multiple - | StorageKeyRequirements::MultipleReferenceToLatest => { - Ok(SimpleConsensusValidationResult::new()) - } - } - } - DECRYPTION => { - // v1 fix: v0 called requires_identity_encryption_bounded_key() here. - let Some(requirements) = document_type - .requires_identity_decryption_bounded_key() - else { - return Ok(SimpleConsensusValidationResult::new_with_error( - ConsensusError::BasicError( - BasicError::DataContractBoundsNotPresentError( - DataContractBoundsNotPresentError::new(*contract_id), - ), - ), - )); - }; - - match requirements { - StorageKeyRequirements::Unique => { - let key_request = IdentityKeysRequest { - identity_id: identity_id.to_buffer(), - request_type: KeyRequestType::ContractDocumentTypeBoundKey(contract_id.to_buffer(), document_type_name.clone(), purpose, KeyKindRequestType::CurrentKeyOfKindRequest), - limit: None, - offset: None, - }; - let maybe_conflicting_key = drive.fetch_identity_keys::(key_request, transaction, platform_version)?; - if let Some(conflicting_key) = maybe_conflicting_key - { - Ok(SimpleConsensusValidationResult::new_with_error(ConsensusError::StateError(StateError::IdentityPublicKeyAlreadyExistsForUniqueContractBoundsError(IdentityPublicKeyAlreadyExistsForUniqueContractBoundsError::new(identity_id, *contract_id, purpose, identity_public_key_in_creation.id(), conflicting_key.id()))))) - } else { - Ok(SimpleConsensusValidationResult::new()) - } - } - StorageKeyRequirements::Multiple - | StorageKeyRequirements::MultipleReferenceToLatest => { - Ok(SimpleConsensusValidationResult::new()) - } - } - } - _ => Ok(SimpleConsensusValidationResult::new_with_error( - ConsensusError::BasicError( - BasicError::InvalidKeyPurposeForContractBoundsError( - InvalidKeyPurposeForContractBoundsError::new( - purpose, - vec![ENCRYPTION, DECRYPTION], - ), - ), - ), - )), - } - } - } - } - } + execution_context.add_operation(ValidationOperation::PrecalculatedOperation(fee)); + if let Some(conflicting_key) = maybe_conflicting_key { + Ok(SimpleConsensusValidationResult::new_with_error( + ConsensusError::StateError( + StateError::IdentityPublicKeyAlreadyExistsForUniqueContractBoundsError( + IdentityPublicKeyAlreadyExistsForUniqueContractBoundsError::new( + identity_id, + contract_id, + purpose, + identity_public_key_in_creation.id(), + conflicting_key.id(), + ), + ), + ), + )) + } else { + Ok(SimpleConsensusValidationResult::new()) } } - } else { - Ok(SimpleConsensusValidationResult::new()) + StorageKeyRequirements::Multiple | StorageKeyRequirements::MultipleReferenceToLatest => { + Ok(SimpleConsensusValidationResult::new()) + } } } diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_update/state/v0/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_update/state/v0/mod.rs index 08997950bf5..92f7c7216d3 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_update/state/v0/mod.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_update/state/v0/mod.rs @@ -1,6 +1,7 @@ use crate::error::Error; use crate::platform_types::platform::PlatformRef; +use crate::platform_types::platform_state::PlatformStateV0Methods; use crate::rpc::core::CoreRPCLike; use dpp::prelude::ConsensusValidationResult; @@ -100,6 +101,7 @@ impl IdentityUpdateStateTransitionStateValidationV0 for IdentityUpdateTransition self.identity_id(), self.public_keys_to_add(), drive, + platform.state.last_committed_block_epoch_ref(), tx, &mut state_transition_execution_context, platform_version, diff --git a/packages/rs-drive/src/drive/identity/key/fetch/fetch_identity_keys/mod.rs b/packages/rs-drive/src/drive/identity/key/fetch/fetch_identity_keys/mod.rs index ec54f15491f..17128768314 100644 --- a/packages/rs-drive/src/drive/identity/key/fetch/fetch_identity_keys/mod.rs +++ b/packages/rs-drive/src/drive/identity/key/fetch/fetch_identity_keys/mod.rs @@ -6,6 +6,8 @@ use crate::error::drive::DriveError; use crate::error::Error; use crate::fees::op::LowLevelDriveOperation; +use dpp::block::epoch::Epoch; +use dpp::fee::fee_result::FeeResult; use dpp::version::PlatformVersion; use grovedb::TransactionArg; @@ -53,6 +55,38 @@ impl Drive { } } + /// Fetch keys matching the request for a specific Identity, along with the fee charged + /// for the underlying grovedb operations. Selects the implementation via the same + /// `fetch_identity_keys` version field as [`Self::fetch_identity_keys`]. + pub fn fetch_identity_keys_with_costs( + &self, + key_request: IdentityKeysRequest, + epoch: &Epoch, + transaction: TransactionArg, + platform_version: &PlatformVersion, + ) -> Result<(T, FeeResult), Error> { + match platform_version + .drive + .methods + .identity + .keys + .fetch + .fetch_identity_keys + { + 0 => self.fetch_identity_keys_with_costs_v0( + key_request, + epoch, + transaction, + platform_version, + ), + version => Err(Error::Drive(DriveError::UnknownVersionMismatch { + method: "fetch_identity_keys_with_costs".to_string(), + known_versions: vec![0], + received: version, + })), + } + } + /// Operations for fetching keys matching the request for a specific Identity /// /// This method fetches the operations that will be used to fetch the requested identity keys. diff --git a/packages/rs-drive/src/drive/identity/key/fetch/fetch_identity_keys/v0/mod.rs b/packages/rs-drive/src/drive/identity/key/fetch/fetch_identity_keys/v0/mod.rs index 18b2d17cf3b..2de3785563c 100644 --- a/packages/rs-drive/src/drive/identity/key/fetch/fetch_identity_keys/v0/mod.rs +++ b/packages/rs-drive/src/drive/identity/key/fetch/fetch_identity_keys/v0/mod.rs @@ -9,6 +9,8 @@ use crate::drive::Drive; use crate::error::Error; use crate::fees::op::LowLevelDriveOperation; +use dpp::block::epoch::Epoch; +use dpp::fee::fee_result::FeeResult; use dpp::version::PlatformVersion; use grovedb::query_result_type::QueryResultType::QueryPathKeyElementTrioResultType; use grovedb::TransactionArg; @@ -30,6 +32,33 @@ impl Drive { ) } + /// Fetch keys matching the request for a specific Identity, along with the fee charged + /// for the underlying grovedb operations. + pub(super) fn fetch_identity_keys_with_costs_v0( + &self, + key_request: IdentityKeysRequest, + epoch: &Epoch, + transaction: TransactionArg, + platform_version: &PlatformVersion, + ) -> Result<(T, FeeResult), Error> { + let mut drive_operations: Vec = vec![]; + let value = self.fetch_identity_keys_operations( + key_request, + transaction, + &mut drive_operations, + platform_version, + )?; + let fees = Drive::calculate_fee( + None, + Some(drive_operations), + epoch, + self.config.epochs_per_era, + platform_version, + None, + )?; + Ok((value, fees)) + } + /// Operations for fetching keys matching the request for a specific Identity pub(super) fn fetch_identity_keys_operations_v0( &self, From 07041229fb7d9856de4a624038ab898bdce2d29c Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Wed, 20 May 2026 20:31:27 +0700 Subject: [PATCH 3/5] =?UTF-8?q?feat(drive):=20add=20get=5Fsystem=5For=5Fus?= =?UTF-8?q?er=5Fcontract=5Fwith=5Ffee=20=E2=80=94=20system=20contracts=20f?= =?UTF-8?q?ree?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bounds validation, and other validators that look up a bound contract, only need to bill grovedb reads when the target is a user contract. System contracts (DPNS, Dashpay, Withdrawals, MasternodeRewards, TokenHistory, KeywordSearch) live in `Drive.cache.system_data_contracts` as an in-memory `ArcSwap`; serving them touches no storage, so no fee should be charged. This commit: - `rs-drive`: new versioned `Drive::get_system_or_user_contract_with_fee` that returns a `FetchedContract` enum: * `FetchedContract::System(Arc)` — served from the in-memory cache, no fee. * `FetchedContract::User { fetch_info, fee }` — fetched from grovedb, `fee` covers the read. Gated by a new `DriveContractGetMethodVersions::get_system_or_user_contract_with_fee` field (initialised to 0 in V1/V2/V3 since this is the first impl). - `rs-drive`: `SystemDataContracts::find_by_id(id)` lookup over the six cached system-contract IDs. - `rs-drive-abci` v1 of bounds validation now uses the new method instead of `get_contract_with_fetch_info_and_fee`. Only the `User` arm pushes a `PrecalculatedOperation` into the execution context; `System` is free. New test `v1_does_not_bill_contract_fetch_for_system_contract` exercises DPNS via `ContractBounds::SingleContract` and asserts the execution context receives zero billing entries (vs. the user-contract case which emits ≥ 2). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../mod.rs | 67 ++++++++++++++ .../v1/mod.rs | 21 ++--- .../rs-drive/src/cache/system_contracts.rs | 26 ++++++ .../mod.rs | 88 +++++++++++++++++++ .../v0/mod.rs | 47 ++++++++++ .../src/drive/contract/get_fetch/mod.rs | 3 + packages/rs-drive/src/drive/contract/mod.rs | 4 +- .../drive_contract_method_versions/mod.rs | 1 + .../drive_contract_method_versions/v1.rs | 1 + .../drive_contract_method_versions/v2.rs | 1 + .../drive_contract_method_versions/v3.rs | 1 + 11 files changed, 249 insertions(+), 11 deletions(-) create mode 100644 packages/rs-drive/src/drive/contract/get_fetch/get_system_or_user_contract_with_fee/mod.rs create mode 100644 packages/rs-drive/src/drive/contract/get_fetch/get_system_or_user_contract_with_fee/v0/mod.rs diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/mod.rs index 7f981da7c65..7036cbc74fa 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/mod.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/mod.rs @@ -230,6 +230,73 @@ mod tests { ); } + #[test] + fn v1_does_not_bill_contract_fetch_for_system_contract() { + // DPNS lives in the in-memory system-contract cache. v1 should resolve it without + // billing a grovedb read — no `PrecalculatedOperation` should be emitted for the + // contract fetch. DPNS doesn't configure bounded keys on any document type, so the + // function returns DataContractBoundsNotPresentError, exercising the "found the + // contract but no requirements" branch. + let platform_version = PlatformVersion::latest(); + let platform = TestPlatformBuilder::new() + .build_with_mock_rpc() + .set_genesis_state(); + + let dpns_id = dpp::system_data_contracts::SystemDataContract::DPNS.id(); + let key = IdentityPublicKeyInCreationV0 { + id: 0, + key_type: KeyType::ECDSA_SECP256K1, + purpose: Purpose::ENCRYPTION, + security_level: SecurityLevel::HIGH, + contract_bounds: Some(ContractBounds::SingleContract { id: dpns_id }), + read_only: false, + data: BinaryData::new(vec![0u8; 33]), + signature: BinaryData::default(), + } + .into(); + + let mut execution_context = + StateTransitionExecutionContext::default_for_platform_version(platform_version) + .expect("execution context"); + let epoch = Epoch::new(0).expect("epoch 0"); + + let result = validate_identity_public_keys_contract_bounds_v1( + Identifier::random(), + &[key], + &platform.drive, + &epoch, + None, + &mut execution_context, + platform_version, + ) + .expect("v1 returns Ok"); + assert!( + !result.is_valid(), + "DPNS configures no bounded keys, so the bound is invalid" + ); + match &result.errors[0] { + ConsensusError::BasicError(BasicError::DataContractBoundsNotPresentError(_)) => {} + other => panic!( + "expected DataContractBoundsNotPresentError, got {:?}", + other + ), + } + + // The critical assertion: no fee was billed for fetching DPNS, because it came from + // the in-memory system-contract cache rather than grovedb. + let billed: Vec<_> = execution_context + .operations_slice() + .iter() + .filter(|op| matches!(op, ValidationOperation::PrecalculatedOperation(_))) + .collect(); + assert!( + billed.is_empty(), + "system contract fetch should incur no fee; got {} billing entries: {:?}", + billed.len(), + billed + ); + } + #[test] fn v1_bills_contract_fetch_and_unique_key_lookup() { // v0 dropped these grovedb-read costs on the floor (audit N6/N7). v1 must push them diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/v1/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/v1/mod.rs index bbcab687776..fa8e940bdd0 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/v1/mod.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/v1/mod.rs @@ -39,6 +39,7 @@ use dpp::state_transition::public_key_in_creation::accessors::IdentityPublicKeyI use dpp::state_transition::public_key_in_creation::IdentityPublicKeyInCreation; use dpp::validation::SimpleConsensusValidationResult; use dpp::version::PlatformVersion; +use drive::drive::contract::FetchedContract; use drive::drive::identity::key::fetch::{ IdentityKeysRequest, KeyKindRequestType, KeyRequestType, OptionalSingleIdentityPublicKeyOutcome, }; @@ -85,29 +86,29 @@ fn validate_identity_public_key_contract_bounds_v1( return Ok(SimpleConsensusValidationResult::new()); }; - // Fetch (and bill) the bounded contract. + // Resolve the bounded contract. System contracts come from the in-memory cache + // (no fee); user contracts come from grovedb with a billed read. let contract_id = match contract_bounds { ContractBounds::SingleContract { id } => *id, ContractBounds::SingleContractDocumentType { id, .. } => *id, }; - let (contract_fee, contract_fetch_info) = drive.get_contract_with_fetch_info_and_fee( + let Some(fetched) = drive.get_system_or_user_contract_with_fee( contract_id.to_buffer(), - Some(epoch), - false, + epoch, transaction, platform_version, - )?; - if let Some(fee) = contract_fee { - execution_context.add_operation(ValidationOperation::PrecalculatedOperation(fee)); - } - let Some(contract_fetch_info) = contract_fetch_info else { + )? + else { return Ok(SimpleConsensusValidationResult::new_with_error( ConsensusError::BasicError(BasicError::DataContractNotPresentError( DataContractNotPresentError::new(contract_id), )), )); }; - let contract = &contract_fetch_info.contract; + if let FetchedContract::User { fee, .. } = &fetched { + execution_context.add_operation(ValidationOperation::PrecalculatedOperation(fee.clone())); + } + let contract = fetched.contract(); match contract_bounds { ContractBounds::SingleContract { .. } => { diff --git a/packages/rs-drive/src/cache/system_contracts.rs b/packages/rs-drive/src/cache/system_contracts.rs index 2bf9f980465..35b0f218e7f 100644 --- a/packages/rs-drive/src/cache/system_contracts.rs +++ b/packages/rs-drive/src/cache/system_contracts.rs @@ -1,6 +1,7 @@ use crate::error::Error; use arc_swap::{ArcSwap, Guard}; use dpp::data_contract::DataContract; +use dpp::prelude::Identifier; use dpp::system_data_contracts::{load_system_data_contract, SystemDataContract}; use platform_version::version::{PlatformVersion, ProtocolVersion}; use std::sync::Arc; @@ -172,4 +173,29 @@ impl SystemDataContracts { pub fn load_keyword_search(&self) -> Guard> { self.keyword_search.load() } + + /// Returns the cached system contract whose deterministic identifier matches `id`, + /// if any. Returns `None` for user contracts and for any system contract whose + /// definition isn't held in this in-memory cache (e.g. `WalletUtils`, which lives + /// only in grovedb). + pub fn find_by_id(&self, id: Identifier) -> Option> { + // Compare against each cached system contract's static `id_bytes`. The match + // is `O(n)` over a small fixed set of variants — cheaper than building a map. + let active = if id == SystemDataContract::Withdrawals.id() { + &self.withdrawals + } else if id == SystemDataContract::MasternodeRewards.id() { + &self.masternode_reward_shares + } else if id == SystemDataContract::DPNS.id() { + &self.dpns + } else if id == SystemDataContract::Dashpay.id() { + &self.dashpay + } else if id == SystemDataContract::TokenHistory.id() { + &self.token_history + } else if id == SystemDataContract::KeywordSearch.id() { + &self.keyword_search + } else { + return None; + }; + Some(Arc::clone(&active.load())) + } } diff --git a/packages/rs-drive/src/drive/contract/get_fetch/get_system_or_user_contract_with_fee/mod.rs b/packages/rs-drive/src/drive/contract/get_fetch/get_system_or_user_contract_with_fee/mod.rs new file mode 100644 index 00000000000..b32969d2a98 --- /dev/null +++ b/packages/rs-drive/src/drive/contract/get_fetch/get_system_or_user_contract_with_fee/mod.rs @@ -0,0 +1,88 @@ +mod v0; + +use crate::drive::contract::DataContractFetchInfo; +use crate::drive::Drive; +use crate::error::drive::DriveError; +use crate::error::Error; +use dpp::block::epoch::Epoch; +use dpp::data_contract::DataContract; +use dpp::fee::fee_result::FeeResult; +use dpp::version::PlatformVersion; +use grovedb::TransactionArg; +use std::sync::Arc; + +/// Outcome of [`Drive::get_system_or_user_contract_with_fee`]. +/// +/// A bound contract is either served from the in-memory system-contract cache (free — +/// no grovedb work) or fetched from storage (billed). Callers that need to bill the +/// underlying read should match `User` and push `fee` into their execution context; +/// `System` is intentionally free. +#[derive(Debug)] +pub enum FetchedContract { + /// Contract was served from the in-memory `SystemDataContracts` cache. No fee. + System(Arc), + /// Contract was fetched from grovedb storage. `fee` covers the read. + User { + /// The fetched contract and its cached fetch metadata. + fetch_info: Arc, + /// The fee charged for the grovedb read that produced `fetch_info`. + fee: FeeResult, + }, +} + +impl FetchedContract { + /// Returns a reference to the underlying `DataContract`, regardless of which path + /// it came from. + pub fn contract(&self) -> &DataContract { + match self { + FetchedContract::System(arc) => arc.as_ref(), + FetchedContract::User { fetch_info, .. } => &fetch_info.contract, + } + } + + /// Returns the read fee for `User`, or `None` for `System` (which is free). + pub fn fee(&self) -> Option<&FeeResult> { + match self { + FetchedContract::System(_) => None, + FetchedContract::User { fee, .. } => Some(fee), + } + } +} + +impl Drive { + /// Resolves `contract_id` either to a system contract from the in-memory cache (free) + /// or to a user contract fetched from grovedb (billed at `epoch`). + /// + /// Returns `Ok(None)` if the id is neither a cached system contract nor present in + /// storage. + /// + /// Selects the implementation via + /// `platform_version.drive.methods.contract.get.get_system_or_user_contract_with_fee`. + pub fn get_system_or_user_contract_with_fee( + &self, + contract_id: [u8; 32], + epoch: &Epoch, + transaction: TransactionArg, + platform_version: &PlatformVersion, + ) -> Result, Error> { + match platform_version + .drive + .methods + .contract + .get + .get_system_or_user_contract_with_fee + { + 0 => self.get_system_or_user_contract_with_fee_v0( + contract_id, + epoch, + transaction, + platform_version, + ), + version => Err(Error::Drive(DriveError::UnknownVersionMismatch { + method: "get_system_or_user_contract_with_fee".to_string(), + known_versions: vec![0], + received: version, + })), + } + } +} diff --git a/packages/rs-drive/src/drive/contract/get_fetch/get_system_or_user_contract_with_fee/v0/mod.rs b/packages/rs-drive/src/drive/contract/get_fetch/get_system_or_user_contract_with_fee/v0/mod.rs new file mode 100644 index 00000000000..bf33549bd7d --- /dev/null +++ b/packages/rs-drive/src/drive/contract/get_fetch/get_system_or_user_contract_with_fee/v0/mod.rs @@ -0,0 +1,47 @@ +use crate::drive::contract::get_fetch::get_system_or_user_contract_with_fee::FetchedContract; +use crate::drive::Drive; +use crate::error::drive::DriveError; +use crate::error::Error; +use dpp::block::epoch::Epoch; +use dpp::prelude::Identifier; +use dpp::version::PlatformVersion; +use grovedb::TransactionArg; + +impl Drive { + /// v0: short-circuit to the in-memory `SystemDataContracts` cache when the id matches a + /// system contract (no fee), otherwise delegate to `get_contract_with_fetch_info_and_fee` + /// at `epoch` (billed). + pub(super) fn get_system_or_user_contract_with_fee_v0( + &self, + contract_id: [u8; 32], + epoch: &Epoch, + transaction: TransactionArg, + platform_version: &PlatformVersion, + ) -> Result, Error> { + if let Some(system_contract) = self + .cache + .system_data_contracts + .find_by_id(Identifier::new(contract_id)) + { + return Ok(Some(FetchedContract::System(system_contract))); + } + + let (fee, fetch_info) = self.get_contract_with_fetch_info_and_fee( + contract_id, + Some(epoch), + false, + transaction, + platform_version, + )?; + match (fetch_info, fee) { + (None, _) => Ok(None), + (Some(fetch_info), Some(fee)) => Ok(Some(FetchedContract::User { fetch_info, fee })), + // `get_contract_with_fetch_info_and_fee` always returns `Some(fee)` when called with + // `Some(epoch)`; reaching this arm indicates an internal invariant violation. + (Some(_), None) => Err(Error::Drive(DriveError::CorruptedCodeExecution( + "get_contract_with_fetch_info_and_fee returned a contract without a fee \ + despite being called with Some(epoch)", + ))), + } + } +} diff --git a/packages/rs-drive/src/drive/contract/get_fetch/mod.rs b/packages/rs-drive/src/drive/contract/get_fetch/mod.rs index 4f94e9e9ec5..21baf8ccbec 100644 --- a/packages/rs-drive/src/drive/contract/get_fetch/mod.rs +++ b/packages/rs-drive/src/drive/contract/get_fetch/mod.rs @@ -5,3 +5,6 @@ mod fetch_contracts; mod get_cached_contract_with_fetch_info; mod get_contract_with_fetch_info; mod get_contracts_with_fetch_info; +pub mod get_system_or_user_contract_with_fee; + +pub use get_system_or_user_contract_with_fee::FetchedContract; diff --git a/packages/rs-drive/src/drive/contract/mod.rs b/packages/rs-drive/src/drive/contract/mod.rs index 8441fb98e40..1e29007ccff 100644 --- a/packages/rs-drive/src/drive/contract/mod.rs +++ b/packages/rs-drive/src/drive/contract/mod.rs @@ -10,7 +10,7 @@ mod contract_fetch_info; #[cfg(feature = "server")] mod estimation_costs; #[cfg(feature = "server")] -mod get_fetch; +pub(crate) mod get_fetch; #[cfg(feature = "server")] mod insert; #[cfg(feature = "server")] @@ -29,6 +29,8 @@ pub mod test_helpers; mod update; #[cfg(feature = "server")] pub use contract_fetch_info::*; +#[cfg(feature = "server")] +pub use get_fetch::FetchedContract; /// How many contracts to fetch at once. This is an arbitrary number and is needed to prevent /// the server from being overloaded with requests. diff --git a/packages/rs-platform-version/src/version/drive_versions/drive_contract_method_versions/mod.rs b/packages/rs-platform-version/src/version/drive_versions/drive_contract_method_versions/mod.rs index da5014455b5..035346101a0 100644 --- a/packages/rs-platform-version/src/version/drive_versions/drive_contract_method_versions/mod.rs +++ b/packages/rs-platform-version/src/version/drive_versions/drive_contract_method_versions/mod.rs @@ -51,6 +51,7 @@ pub struct DriveContractGetMethodVersions { pub get_cached_contract_with_fetch_info: FeatureVersion, pub get_contract_with_fetch_info: FeatureVersion, pub get_contracts_with_fetch_info: FeatureVersion, + pub get_system_or_user_contract_with_fee: FeatureVersion, } #[derive(Clone, Debug, Default)] diff --git a/packages/rs-platform-version/src/version/drive_versions/drive_contract_method_versions/v1.rs b/packages/rs-platform-version/src/version/drive_versions/drive_contract_method_versions/v1.rs index b8a8757e6ec..50dfad29f2c 100644 --- a/packages/rs-platform-version/src/version/drive_versions/drive_contract_method_versions/v1.rs +++ b/packages/rs-platform-version/src/version/drive_versions/drive_contract_method_versions/v1.rs @@ -37,5 +37,6 @@ pub const DRIVE_CONTRACT_METHOD_VERSIONS_V1: DriveContractMethodVersions = get_cached_contract_with_fetch_info: 0, get_contract_with_fetch_info: 0, get_contracts_with_fetch_info: 0, + get_system_or_user_contract_with_fee: 0, }, }; diff --git a/packages/rs-platform-version/src/version/drive_versions/drive_contract_method_versions/v2.rs b/packages/rs-platform-version/src/version/drive_versions/drive_contract_method_versions/v2.rs index 802e15e8f13..3f2334e29cf 100644 --- a/packages/rs-platform-version/src/version/drive_versions/drive_contract_method_versions/v2.rs +++ b/packages/rs-platform-version/src/version/drive_versions/drive_contract_method_versions/v2.rs @@ -37,5 +37,6 @@ pub const DRIVE_CONTRACT_METHOD_VERSIONS_V2: DriveContractMethodVersions = get_cached_contract_with_fetch_info: 0, get_contract_with_fetch_info: 0, get_contracts_with_fetch_info: 0, + get_system_or_user_contract_with_fee: 0, }, }; diff --git a/packages/rs-platform-version/src/version/drive_versions/drive_contract_method_versions/v3.rs b/packages/rs-platform-version/src/version/drive_versions/drive_contract_method_versions/v3.rs index 2cf0723baef..dde31a3e10e 100644 --- a/packages/rs-platform-version/src/version/drive_versions/drive_contract_method_versions/v3.rs +++ b/packages/rs-platform-version/src/version/drive_versions/drive_contract_method_versions/v3.rs @@ -48,5 +48,6 @@ pub const DRIVE_CONTRACT_METHOD_VERSIONS_V3: DriveContractMethodVersions = get_cached_contract_with_fetch_info: 0, get_contract_with_fetch_info: 0, get_contracts_with_fetch_info: 0, + get_system_or_user_contract_with_fee: 0, }, }; From 616d741b3e3ae3ff8ff636b1e400f42f4c5c3701 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Wed, 20 May 2026 20:51:45 +0700 Subject: [PATCH 4/5] fix(drive): bill the grovedb lookup even when contract is not found MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `get_system_or_user_contract_with_fee_v0` was returning `Ok(None)` for the not-found case, dropping the fee from the failed grovedb probe on the floor. The probe itself still ran and still cost something. Reshaped the return type: - Removed the outer `Option`. - Renamed `FetchedContract` → `ContractFetchOutcome` and added a `NotFound { fee: FeeResult }` variant alongside `System(...)` and `User { fetch_info, fee }`. - `ContractFetchOutcome::fee()` now returns `Some(&fee)` for both `User` and `NotFound` (and `None` only for `System`, which never touches storage). `contract()` returns `Option<&DataContract>` — `None` only for `NotFound`. Updated v1 of bounds validation accordingly: it bills `outcome.fee()` unconditionally, then checks `outcome.contract()` for the not-present path. The new flow naturally bills the probe even for missing contracts. New test `v1_bills_lookup_when_contract_not_found` exercises a random non-system contract id and asserts the execution context receives exactly one `PrecalculatedOperation` with a non-zero processing fee, plus the expected `DataContractNotPresentError`. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../mod.rs | 70 +++++++++++++++++++ .../v1/mod.rs | 17 +++-- .../mod.rs | 50 +++++++------ .../v0/mod.rs | 30 ++++---- .../src/drive/contract/get_fetch/mod.rs | 2 +- packages/rs-drive/src/drive/contract/mod.rs | 2 +- 6 files changed, 127 insertions(+), 44 deletions(-) diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/mod.rs index 7036cbc74fa..1f3c6bcb9a9 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/mod.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/mod.rs @@ -365,4 +365,74 @@ mod tests { precalculated ); } + + #[test] + fn v1_bills_lookup_when_contract_not_found() { + // A random (non-system) contract id will miss both the system-contract cache and + // storage. The grovedb lookup that determined absence still costs the caller — + // assert that v1 bills it via `ContractFetchOutcome::NotFound { fee }`. + let platform_version = PlatformVersion::latest(); + let platform = TestPlatformBuilder::new() + .build_with_mock_rpc() + .set_genesis_state(); + + let missing_contract_id = Identifier::random(); + let key = IdentityPublicKeyInCreationV0 { + id: 0, + key_type: KeyType::ECDSA_SECP256K1, + purpose: Purpose::ENCRYPTION, + security_level: SecurityLevel::HIGH, + contract_bounds: Some(ContractBounds::SingleContract { + id: missing_contract_id, + }), + read_only: false, + data: BinaryData::new(vec![0u8; 33]), + signature: BinaryData::default(), + } + .into(); + + let mut execution_context = + StateTransitionExecutionContext::default_for_platform_version(platform_version) + .expect("execution context"); + let epoch = Epoch::new(0).expect("epoch 0"); + + let result = validate_identity_public_keys_contract_bounds_v1( + Identifier::random(), + &[key], + &platform.drive, + &epoch, + None, + &mut execution_context, + platform_version, + ) + .expect("v1 returns Ok"); + + assert!(!result.is_valid(), "missing contract should be invalid"); + match &result.errors[0] { + ConsensusError::BasicError(BasicError::DataContractNotPresentError(e)) => { + assert_eq!(e.data_contract_id(), &missing_contract_id); + } + other => panic!("expected DataContractNotPresentError, got {:?}", other), + } + + let precalculated: Vec<_> = execution_context + .operations_slice() + .iter() + .filter_map(|op| match op { + ValidationOperation::PrecalculatedOperation(fee) => Some(fee), + _ => None, + }) + .collect(); + assert_eq!( + precalculated.len(), + 1, + "expected exactly one billed entry (the failed grovedb lookup); got: {:?}", + precalculated + ); + assert!( + precalculated[0].processing_fee > 0, + "expected the not-found lookup to incur a non-zero processing fee; got {:?}", + precalculated[0] + ); + } } diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/v1/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/v1/mod.rs index fa8e940bdd0..57a8c428d9d 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/v1/mod.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/v1/mod.rs @@ -39,7 +39,6 @@ use dpp::state_transition::public_key_in_creation::accessors::IdentityPublicKeyI use dpp::state_transition::public_key_in_creation::IdentityPublicKeyInCreation; use dpp::validation::SimpleConsensusValidationResult; use dpp::version::PlatformVersion; -use drive::drive::contract::FetchedContract; use drive::drive::identity::key::fetch::{ IdentityKeysRequest, KeyKindRequestType, KeyRequestType, OptionalSingleIdentityPublicKeyOutcome, }; @@ -87,28 +86,28 @@ fn validate_identity_public_key_contract_bounds_v1( }; // Resolve the bounded contract. System contracts come from the in-memory cache - // (no fee); user contracts come from grovedb with a billed read. + // (no fee); user contracts come from grovedb (billed); a missing contract still + // costs the grovedb lookup that proved its absence (also billed). let contract_id = match contract_bounds { ContractBounds::SingleContract { id } => *id, ContractBounds::SingleContractDocumentType { id, .. } => *id, }; - let Some(fetched) = drive.get_system_or_user_contract_with_fee( + let outcome = drive.get_system_or_user_contract_with_fee( contract_id.to_buffer(), epoch, transaction, platform_version, - )? - else { + )?; + if let Some(fee) = outcome.fee() { + execution_context.add_operation(ValidationOperation::PrecalculatedOperation(fee.clone())); + } + let Some(contract) = outcome.contract() else { return Ok(SimpleConsensusValidationResult::new_with_error( ConsensusError::BasicError(BasicError::DataContractNotPresentError( DataContractNotPresentError::new(contract_id), )), )); }; - if let FetchedContract::User { fee, .. } = &fetched { - execution_context.add_operation(ValidationOperation::PrecalculatedOperation(fee.clone())); - } - let contract = fetched.contract(); match contract_bounds { ContractBounds::SingleContract { .. } => { diff --git a/packages/rs-drive/src/drive/contract/get_fetch/get_system_or_user_contract_with_fee/mod.rs b/packages/rs-drive/src/drive/contract/get_fetch/get_system_or_user_contract_with_fee/mod.rs index b32969d2a98..6644a309135 100644 --- a/packages/rs-drive/src/drive/contract/get_fetch/get_system_or_user_contract_with_fee/mod.rs +++ b/packages/rs-drive/src/drive/contract/get_fetch/get_system_or_user_contract_with_fee/mod.rs @@ -13,12 +13,13 @@ use std::sync::Arc; /// Outcome of [`Drive::get_system_or_user_contract_with_fee`]. /// -/// A bound contract is either served from the in-memory system-contract cache (free — -/// no grovedb work) or fetched from storage (billed). Callers that need to bill the -/// underlying read should match `User` and push `fee` into their execution context; -/// `System` is intentionally free. +/// A lookup either hits the in-memory system-contract cache (free, no grovedb work), +/// hits storage and finds a user contract (billed), or hits storage and finds nothing +/// (also billed — the grovedb lookup still ran). Callers that need to bill the +/// underlying read should call [`Self::fee`] / [`Self::contract`] or match the variant +/// directly. #[derive(Debug)] -pub enum FetchedContract { +pub enum ContractFetchOutcome { /// Contract was served from the in-memory `SystemDataContracts` cache. No fee. System(Arc), /// Contract was fetched from grovedb storage. `fee` covers the read. @@ -28,33 +29,42 @@ pub enum FetchedContract { /// The fee charged for the grovedb read that produced `fetch_info`. fee: FeeResult, }, + /// `contract_id` is not a system contract and storage has no entry for it. The + /// grovedb lookup that determined this still costs the caller — `fee` covers it. + NotFound { + /// The fee charged for the grovedb lookup that returned no contract. + fee: FeeResult, + }, } -impl FetchedContract { - /// Returns a reference to the underlying `DataContract`, regardless of which path - /// it came from. - pub fn contract(&self) -> &DataContract { +impl ContractFetchOutcome { + /// Returns a reference to the underlying `DataContract`, or `None` for `NotFound`. + pub fn contract(&self) -> Option<&DataContract> { match self { - FetchedContract::System(arc) => arc.as_ref(), - FetchedContract::User { fetch_info, .. } => &fetch_info.contract, + ContractFetchOutcome::System(arc) => Some(arc.as_ref()), + ContractFetchOutcome::User { fetch_info, .. } => Some(&fetch_info.contract), + ContractFetchOutcome::NotFound { .. } => None, } } - /// Returns the read fee for `User`, or `None` for `System` (which is free). + /// Returns the fee for the lookup that produced this outcome: + /// - `None` for `System` (served from in-memory cache, no grovedb work). + /// - `Some(&fee)` for `User` (cost of the read that returned the contract). + /// - `Some(&fee)` for `NotFound` (cost of the read that proved absence). pub fn fee(&self) -> Option<&FeeResult> { match self { - FetchedContract::System(_) => None, - FetchedContract::User { fee, .. } => Some(fee), + ContractFetchOutcome::System(_) => None, + ContractFetchOutcome::User { fee, .. } => Some(fee), + ContractFetchOutcome::NotFound { fee } => Some(fee), } } } impl Drive { - /// Resolves `contract_id` either to a system contract from the in-memory cache (free) - /// or to a user contract fetched from grovedb (billed at `epoch`). - /// - /// Returns `Ok(None)` if the id is neither a cached system contract nor present in - /// storage. + /// Resolves `contract_id` to one of three outcomes: + /// - a system contract from the in-memory cache (no fee), + /// - a user contract fetched from grovedb (billed at `epoch`), + /// - or absence (still billed — the grovedb lookup ran). /// /// Selects the implementation via /// `platform_version.drive.methods.contract.get.get_system_or_user_contract_with_fee`. @@ -64,7 +74,7 @@ impl Drive { epoch: &Epoch, transaction: TransactionArg, platform_version: &PlatformVersion, - ) -> Result, Error> { + ) -> Result { match platform_version .drive .methods diff --git a/packages/rs-drive/src/drive/contract/get_fetch/get_system_or_user_contract_with_fee/v0/mod.rs b/packages/rs-drive/src/drive/contract/get_fetch/get_system_or_user_contract_with_fee/v0/mod.rs index bf33549bd7d..35bf53a6d4d 100644 --- a/packages/rs-drive/src/drive/contract/get_fetch/get_system_or_user_contract_with_fee/v0/mod.rs +++ b/packages/rs-drive/src/drive/contract/get_fetch/get_system_or_user_contract_with_fee/v0/mod.rs @@ -1,4 +1,4 @@ -use crate::drive::contract::get_fetch::get_system_or_user_contract_with_fee::FetchedContract; +use crate::drive::contract::get_fetch::get_system_or_user_contract_with_fee::ContractFetchOutcome; use crate::drive::Drive; use crate::error::drive::DriveError; use crate::error::Error; @@ -10,20 +10,20 @@ use grovedb::TransactionArg; impl Drive { /// v0: short-circuit to the in-memory `SystemDataContracts` cache when the id matches a /// system contract (no fee), otherwise delegate to `get_contract_with_fetch_info_and_fee` - /// at `epoch` (billed). + /// at `epoch` (billed — including when the contract is absent, since the lookup still ran). pub(super) fn get_system_or_user_contract_with_fee_v0( &self, contract_id: [u8; 32], epoch: &Epoch, transaction: TransactionArg, platform_version: &PlatformVersion, - ) -> Result, Error> { + ) -> Result { if let Some(system_contract) = self .cache .system_data_contracts .find_by_id(Identifier::new(contract_id)) { - return Ok(Some(FetchedContract::System(system_contract))); + return Ok(ContractFetchOutcome::System(system_contract)); } let (fee, fetch_info) = self.get_contract_with_fetch_info_and_fee( @@ -33,15 +33,19 @@ impl Drive { transaction, platform_version, )?; - match (fetch_info, fee) { - (None, _) => Ok(None), - (Some(fetch_info), Some(fee)) => Ok(Some(FetchedContract::User { fetch_info, fee })), - // `get_contract_with_fetch_info_and_fee` always returns `Some(fee)` when called with - // `Some(epoch)`; reaching this arm indicates an internal invariant violation. - (Some(_), None) => Err(Error::Drive(DriveError::CorruptedCodeExecution( - "get_contract_with_fetch_info_and_fee returned a contract without a fee \ - despite being called with Some(epoch)", - ))), + // `get_contract_with_fetch_info_and_fee` populates `fee_result` from the + // accumulated drive operations whenever an epoch is supplied — so `fee` + // is `Some(...)` for both the found and not-found cases. A `None` fee + // would indicate an internal invariant violation. + let Some(fee) = fee else { + return Err(Error::Drive(DriveError::CorruptedCodeExecution( + "get_contract_with_fetch_info_and_fee returned no fee despite being \ + called with Some(epoch)", + ))); + }; + match fetch_info { + Some(fetch_info) => Ok(ContractFetchOutcome::User { fetch_info, fee }), + None => Ok(ContractFetchOutcome::NotFound { fee }), } } } diff --git a/packages/rs-drive/src/drive/contract/get_fetch/mod.rs b/packages/rs-drive/src/drive/contract/get_fetch/mod.rs index 21baf8ccbec..be0af692c34 100644 --- a/packages/rs-drive/src/drive/contract/get_fetch/mod.rs +++ b/packages/rs-drive/src/drive/contract/get_fetch/mod.rs @@ -7,4 +7,4 @@ mod get_contract_with_fetch_info; mod get_contracts_with_fetch_info; pub mod get_system_or_user_contract_with_fee; -pub use get_system_or_user_contract_with_fee::FetchedContract; +pub use get_system_or_user_contract_with_fee::ContractFetchOutcome; diff --git a/packages/rs-drive/src/drive/contract/mod.rs b/packages/rs-drive/src/drive/contract/mod.rs index 1e29007ccff..7c3e5aae88e 100644 --- a/packages/rs-drive/src/drive/contract/mod.rs +++ b/packages/rs-drive/src/drive/contract/mod.rs @@ -30,7 +30,7 @@ mod update; #[cfg(feature = "server")] pub use contract_fetch_info::*; #[cfg(feature = "server")] -pub use get_fetch::FetchedContract; +pub use get_fetch::ContractFetchOutcome; /// How many contracts to fetch at once. This is an arbitrary number and is needed to prevent /// the server from being overloaded with requests. From 79b259debf74ad2a9826c86ac670ab24b5cdbd6c Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Wed, 20 May 2026 22:35:29 +0700 Subject: [PATCH 5/5] test(drive-abci): exercise the bounds dispatcher entrypoint (addresses CodeRabbit nitpick) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prior tests called validate_identity_public_keys_contract_bounds_v0 / _v1 directly and would not catch a regression in the v1 dispatcher arm or the new `epoch` forwarding. Adds `dispatcher_routes_to_v1_at_latest_platform_version`, which: - Sanity-checks that PlatformVersion::latest() selects v1 of the bounds-validator field (test premise; fails loudly if the version field moves). - Calls the public `validate_identity_public_keys_contract_bounds` dispatcher against the asymmetric-decryption-bounds contract used by the v0/v1 differential test, and asserts the result is valid — proving the dispatcher routed to v1 (v0 would return DataContractBoundsNotPresentError here per the existing pinned-v0 test). - Asserts the execution context received ≥ 1 PrecalculatedOperation entry, proving the epoch parameter is forwarded through to v1 (v0 wouldn't bill). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../mod.rs | 76 +++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/mod.rs index 1f3c6bcb9a9..62575b8cfb4 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/mod.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/common/validate_identity_public_key_contract_bounds/mod.rs @@ -74,6 +74,7 @@ mod tests { //! v0's behavior is frozen for chain replay — the v0 assertion below pins it. use super::v0::validate_identity_public_keys_contract_bounds_v0; use super::v1::validate_identity_public_keys_contract_bounds_v1; + use super::validate_identity_public_keys_contract_bounds; use crate::execution::types::execution_operation::ValidationOperation; use crate::execution::types::state_transition_execution_context::{ StateTransitionExecutionContext, StateTransitionExecutionContextMethodsV0, @@ -435,4 +436,79 @@ mod tests { precalculated[0] ); } + + /// Covers the integration this PR is wiring up — that the public dispatcher actually + /// routes to v1 under `PlatformVersion::latest()` (which sets the bounds-validator + /// version field to 1) and that the `epoch` parameter is forwarded through. If the + /// dispatcher were accidentally routing to v0 — which has the DECRYPTION-branch bug — + /// the assertion below would flip from `is_valid` to invalid. + #[test] + fn dispatcher_routes_to_v1_at_latest_platform_version() { + let platform_version = PlatformVersion::latest(); + // Sanity: `latest` should select v1 of the bounds validator. + assert_eq!( + platform_version + .drive_abci + .validation_and_processing + .state_transitions + .common_validation_methods + .validate_identity_public_key_contract_bounds, + 1, + "test premise: latest platform version is expected to select v1; \ + update this test if the version field moves" + ); + + let platform = TestPlatformBuilder::new() + .build_with_mock_rpc() + .set_genesis_state(); + + let contract = build_contract_with_decryption_only_bounds(platform_version); + let contract_id = contract.id(); + platform + .drive + .apply_contract( + &contract, + BlockInfo::default(), + true, + None, + None, + platform_version, + ) + .expect("contract applied"); + + let key = make_decryption_key_bound_to_doc_type(contract_id, "note".to_string()); + let mut execution_context = + StateTransitionExecutionContext::default_for_platform_version(platform_version) + .expect("execution context"); + let epoch = Epoch::new(0).expect("epoch 0"); + + let result = validate_identity_public_keys_contract_bounds( + Identifier::random(), + &[key], + &platform.drive, + &epoch, + None, + &mut execution_context, + platform_version, + ) + .expect("dispatcher returns Ok"); + + assert!( + result.is_valid(), + "dispatcher routed away from v1 (or v1 regressed); got errors: {:?}", + result.errors + ); + // Forwarded epoch reaches v1 → billing happens. ≥ 1 entry proves the epoch + // parameter is not being dropped en route. + let billed_count = execution_context + .operations_slice() + .iter() + .filter(|op| matches!(op, ValidationOperation::PrecalculatedOperation(_))) + .count(); + assert!( + billed_count >= 1, + "dispatcher must forward epoch to v1 so reads get billed; got {} entries", + billed_count + ); + } }