diff --git a/docs/protocol/contract-bound-authentication-keys.md b/docs/protocol/contract-bound-authentication-keys.md index 19ff68adada..80563c1b5ac 100644 --- a/docs/protocol/contract-bound-authentication-keys.md +++ b/docs/protocol/contract-bound-authentication-keys.md @@ -58,9 +58,10 @@ level do not already allow. - A group-bound key may sign only Batch transitions. A member on contract `C` is inside the bounds when `C` is a whole-contract member of the group, when the member's document type is a member of the group, or when the member's token is a member of the group. - Consensus reads `C`'s group memberships once per batch member and bills the read; a - member outside the group fails as a paid `ContractBoundedKeyOutOfBoundsError`, as for - a contract bound. + Consensus reads the group memberships of each distinct contract the batch touches + once, however many members name that contract, and bills each read once; a member + outside the group fails as a paid `ContractBoundedKeyOutOfBoundsError`, as for a + contract bound. - Memberships are append-only, so what a group-bound key may sign grows whenever the group's owner or an admin adds a contract, document type or token. Binding a key to a group trusts the group's owner and admins with that growth. diff --git a/docs/protocol/contract-groups.md b/docs/protocol/contract-groups.md index feaf6e52674..16335b69cc5 100644 --- a/docs/protocol/contract-groups.md +++ b/docs/protocol/contract-groups.md @@ -132,8 +132,8 @@ the responses into `ContractGroupInfo`, `ContractGroupMembersPage` and An identity's AUTHENTICATION key may carry `contractBounds` of type `contractGroup` naming a group. The key may then sign batch members whose contract, document type or -token is a member of the group; consensus reads the member contract's memberships once -per batch member and bills the read. The group must exist when the key is registered, -any identity may bind a key to any group, and encryption and decryption keys cannot be -group-bound. `IdentityCreateFromShieldedPool` refuses group-bound keys. The rules and -errors are in `contract-bound-authentication-keys.md`. +token is a member of the group; consensus reads the memberships of each distinct +contract the batch touches once and bills each read once. The group must exist when the +key is registered, any identity may bind a key to any group, and encryption and +decryption keys cannot be group-bound. `IdentityCreateFromShieldedPool` refuses +group-bound keys. The rules and errors are in `contract-bound-authentication-keys.md`. diff --git a/packages/rs-dpp/src/identity/identity_public_key/contract_bounds/mod.rs b/packages/rs-dpp/src/identity/identity_public_key/contract_bounds/mod.rs index 41097285e56..80af5f6ccb5 100644 --- a/packages/rs-dpp/src/identity/identity_public_key/contract_bounds/mod.rs +++ b/packages/rs-dpp/src/identity/identity_public_key/contract_bounds/mod.rs @@ -1,3 +1,4 @@ +#[cfg(feature = "state-transitions")] use crate::contract_group::ContractGroupMember; use crate::identifier::Identifier; use crate::identity::identity_public_key::contract_bounds::ContractBounds::{ @@ -416,7 +417,7 @@ mod tests { } #[test] - fn contract_bounds_contract_group_json_round_trip() { + fn should_round_trip_contract_group_bounds_through_json() { let id = Identifier::from([0xEFu8; 32]); let bounds = ContractBounds::ContractGroup { id }; diff --git a/packages/rs-dpp/src/shielded/builder/identity_create_from_shielded_pool.rs b/packages/rs-dpp/src/shielded/builder/identity_create_from_shielded_pool.rs index e0e08055f3b..f3e805c78f5 100644 --- a/packages/rs-dpp/src/shielded/builder/identity_create_from_shielded_pool.rs +++ b/packages/rs-dpp/src/shielded/builder/identity_create_from_shielded_pool.rs @@ -3,7 +3,6 @@ use grovedb_commitment_tree::{Anchor, FullViewingKey, SpendAuthorizingKey}; use crate::address_funds::OrchardAddress; use crate::address_funds::PlatformAddress; use crate::fee::Credits; -use crate::identity::contract_bounds::ContractBounds; use crate::identity::identity_public_key::accessors::v0::IdentityPublicKeyGettersV0; use crate::identity::signer::Signer; use crate::identity::IdentityPublicKey; @@ -187,12 +186,9 @@ where let mut bound_identity_id: Option = None; // Consensus refuses a key bound to a contract group in this transition (its Orchard sighash // layout predates group bounds); refuse it here before a proof is generated. - if let Some(key) = in_creation_keys.iter().find(|key| { - matches!( - key.contract_bounds(), - Some(ContractBounds::ContractGroup { .. }) - ) - }) { + if let Some(key) = + IdentityPublicKeyInCreation::first_bound_to_a_contract_group(&in_creation_keys) + { return Err(ProtocolError::ShieldedBuildError(format!( "key {} is bound to a contract group, which an identity created from the shielded \ pool cannot register; add it with an identity update", diff --git a/packages/rs-dpp/src/state_transition/mod.rs b/packages/rs-dpp/src/state_transition/mod.rs index 848e2dad70b..9f2643b106d 100644 --- a/packages/rs-dpp/src/state_transition/mod.rs +++ b/packages/rs-dpp/src/state_transition/mod.rs @@ -70,7 +70,6 @@ use crate::fee::Credits; use crate::identity::identity_public_key::accessors::v0::IdentityPublicKeyGettersV0; #[cfg(feature = "state-transition-signing")] use crate::identity::identity_public_key::contract_bounds::BatchedTransitionBoundsCheck; -use crate::identity::identity_public_key::contract_bounds::ContractBounds; #[cfg(feature = "state-transition-signing")] use crate::identity::signer::Signer; use crate::identity::state_transition::OptionallyAssetLockProved; @@ -152,7 +151,6 @@ use crate::state_transition::identity_update_transition::{ }; use crate::state_transition::masternode_vote_transition::MasternodeVoteTransition; use crate::state_transition::masternode_vote_transition::MasternodeVoteTransitionSignable; -use crate::state_transition::public_key_in_creation::accessors::IdentityPublicKeyInCreationV0Getters; use crate::state_transition::public_key_in_creation::IdentityPublicKeyInCreation; use crate::state_transition::shield_from_asset_lock_transition::{ ShieldFromAssetLockTransition, ShieldFromAssetLockTransitionSignable, @@ -860,12 +858,7 @@ fn active_version_range_for_keys_in_creation( keys: &[IdentityPublicKeyInCreation], otherwise: RangeInclusive, ) -> RangeInclusive { - if keys.iter().any(|key| { - matches!( - key.contract_bounds(), - Some(ContractBounds::ContractGroup { .. }) - ) - }) { + if IdentityPublicKeyInCreation::first_bound_to_a_contract_group(keys).is_some() { 14..=LATEST_VERSION } else { otherwise @@ -3475,6 +3468,38 @@ mod tests { // A version 1 data contract create carries contract groups, which only exist from // protocol version 14. Below that a node must reject it rather than create the // contract and drop the group data. + #[test] + fn test_data_contract_create_v1_is_not_active_before_protocol_version_14() { + use crate::serialization::PlatformSerializable; + + let original = sample_data_contract_create_v1_st(); + assert_eq!(original.active_version_range(), 14..=LATEST_VERSION); + + let bytes = + PlatformSerializable::serialize_to_bytes(&original).expect("serialize succeeds"); + + let version_13 = PlatformVersion::get(13).expect("platform version 13 exists"); + let err = StateTransition::deserialize_from_bytes_untrusted_in_version(&bytes, version_13) + .expect_err("expected StateTransitionIsNotActiveError at protocol version 13"); + match err { + ProtocolError::StateTransitionError( + crate::state_transition::errors::StateTransitionError::StateTransitionIsNotActiveError { + active_version_range, + current_protocol_version, + .. + }, + ) => { + assert_eq!(current_protocol_version, 13); + assert_eq!(*active_version_range.start(), 14); + } + other => panic!("expected StateTransitionIsNotActiveError, got {other:?}"), + } + + let version_14 = PlatformVersion::get(14).expect("platform version 14 exists"); + StateTransition::deserialize_from_bytes_untrusted_in_version(&bytes, version_14) + .expect("a version 1 create is active at protocol version 14"); + } + #[test] fn should_gate_identity_transitions_carrying_a_contract_group_bound_key_to_protocol_version_14() { @@ -3555,38 +3580,6 @@ mod tests { .expect("a contract group bound key is active at protocol version 14"); } - #[test] - fn test_data_contract_create_v1_is_not_active_before_protocol_version_14() { - use crate::serialization::PlatformSerializable; - - let original = sample_data_contract_create_v1_st(); - assert_eq!(original.active_version_range(), 14..=LATEST_VERSION); - - let bytes = - PlatformSerializable::serialize_to_bytes(&original).expect("serialize succeeds"); - - let version_13 = PlatformVersion::get(13).expect("platform version 13 exists"); - let err = StateTransition::deserialize_from_bytes_untrusted_in_version(&bytes, version_13) - .expect_err("expected StateTransitionIsNotActiveError at protocol version 13"); - match err { - ProtocolError::StateTransitionError( - crate::state_transition::errors::StateTransitionError::StateTransitionIsNotActiveError { - active_version_range, - current_protocol_version, - .. - }, - ) => { - assert_eq!(current_protocol_version, 13); - assert_eq!(*active_version_range.start(), 14); - } - other => panic!("expected StateTransitionIsNotActiveError, got {other:?}"), - } - - let version_14 = PlatformVersion::get(14).expect("platform version 14 exists"); - StateTransition::deserialize_from_bytes_untrusted_in_version(&bytes, version_14) - .expect("a version 1 create is active at protocol version 14"); - } - // ----------------------------------------------------------------------- // Additional coverage: variants not yet exercised. // diff --git a/packages/rs-dpp/src/state_transition/state_transitions/identity/public_key_in_creation/mod.rs b/packages/rs-dpp/src/state_transition/state_transitions/identity/public_key_in_creation/mod.rs index 05113aa9cff..3b99accd280 100644 --- a/packages/rs-dpp/src/state_transition/state_transitions/identity/public_key_in_creation/mod.rs +++ b/packages/rs-dpp/src/state_transition/state_transitions/identity/public_key_in_creation/mod.rs @@ -1,8 +1,10 @@ +use crate::identity::contract_bounds::ContractBounds; use crate::identity::IdentityPublicKey; #[cfg(feature = "json-conversion")] use crate::serialization::JsonConvertible; #[cfg(feature = "value-conversion")] use crate::serialization::ValueConvertible; +use crate::state_transition::public_key_in_creation::accessors::IdentityPublicKeyInCreationV0Getters; use crate::state_transition::public_key_in_creation::v0::IdentityPublicKeyInCreationV0; use crate::state_transition::public_key_in_creation::v0::IdentityPublicKeyInCreationV0Signable; use crate::ProtocolError; @@ -54,6 +56,17 @@ impl IdentityPublicKeyInCreation { }), } } + + /// The first of `keys` bound to a contract group, if any. A transition carrying such a key + /// is active from protocol version 14, and an identity created from the shielded pool + /// cannot register one. + pub fn first_bound_to_a_contract_group(keys: &[Self]) -> Option<&Self> { + keys.iter().find(|key| { + key.contract_bounds() + .and_then(ContractBounds::contract_group_id) + .is_some() + }) + } } impl From<&IdentityPublicKeyInCreation> for IdentityPublicKey { 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 06578e4af36..1d72819e86f 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 @@ -91,7 +91,9 @@ mod tests { use crate::execution::types::state_transition_execution_context::{ StateTransitionExecutionContext, StateTransitionExecutionContextMethodsV0, }; - use crate::test::helpers::setup::TestPlatformBuilder; + use crate::rpc::core::MockCoreRPCLike; + use crate::test::helpers::contract_groups::{register_group, single_owner_info}; + use crate::test::helpers::setup::{TempPlatform, TestPlatformBuilder}; use dpp::block::block_info::BlockInfo; use dpp::block::epoch::Epoch; use dpp::consensus::basic::BasicError; @@ -619,31 +621,16 @@ mod tests { fn platform_with_contract_group( contract_group_id: Identifier, - ) -> crate::test::helpers::setup::TempPlatform { - use dpp::contract_group::{ContractGroupInfo, ContractGroupRegistration}; + ) -> TempPlatform { let platform = TestPlatformBuilder::new() .build_with_mock_rpc() .set_genesis_state(); - let info: ContractGroupInfo = ( - Identifier::from([0x60; 32]), - ContractGroupRegistration { - admins: Default::default(), - name: None, - description: None, - }, - ) - .into(); - platform - .drive - .insert_contract_group( - contract_group_id, - &info, - &BlockInfo::default(), - true, - None, - PlatformVersion::latest(), - ) - .expect("expected to register the group"); + register_group( + &platform, + contract_group_id, + &single_owner_info(Identifier::from([0x60; 32]), None, None), + PlatformVersion::latest(), + ); platform } diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/processor/traits/shielded_proof.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/processor/traits/shielded_proof.rs index c187e8e7b50..368f33583d4 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/processor/traits/shielded_proof.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/processor/traits/shielded_proof.rs @@ -14,7 +14,6 @@ use dpp::consensus::basic::BasicError; use dpp::consensus::state::shielded::insufficient_shielded_fee_error::InsufficientShieldedFeeError; use dpp::consensus::state::state_error::StateError; use dpp::consensus::ConsensusError; -use dpp::identity::contract_bounds::ContractBounds; use dpp::serialization::{PlatformMessageSignable, Signable}; use dpp::state_transition::public_key_in_creation::accessors::IdentityPublicKeyInCreationV0Getters; use dpp::state_transition::public_key_in_creation::IdentityPublicKeyInCreation; @@ -678,17 +677,9 @@ fn contract_group_bound_key_in_shielded_creation( return None; }; let IdentityCreateFromShieldedPoolTransition::V0(v0) = st; - v0.public_keys - .iter() - .find(|key| { - matches!( - key.contract_bounds(), - Some(ContractBounds::ContractGroup { .. }) - ) - }) - .map(|key| { - ContractGroupBoundKeyNotAllowedInShieldedIdentityCreationError::new(key.id()).into() - }) + IdentityPublicKeyInCreation::first_bound_to_a_contract_group(&v0.public_keys).map(|key| { + ContractGroupBoundKeyNotAllowedInShieldedIdentityCreationError::new(key.id()).into() + }) } #[cfg(test)] diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/state/v2/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/state/v2/mod.rs index 70916d5e433..869f5fc2657 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/state/v2/mod.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/state/v2/mod.rs @@ -78,10 +78,13 @@ impl DocumentsBatchStateTransitionStateValidationV2 for BatchTransition { execution_context, )?; + // A result with errors never reaches the bounds check, so nothing is resolved for it. + let bounds_will_be_checked = + signed_by_a_group_bound_key && validation_result.errors.is_empty(); if let Some(action) = validation_result .data .as_mut() - .filter(|_| signed_by_a_group_bound_key) + .filter(|_| bounds_will_be_checked) { let platform_version = platform.state.current_platform_version()?; let contract_ids: BTreeSet = self diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/contract_group_bound_auth.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/contract_group_bound_auth.rs index 8c072daa0f2..55146a876c1 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/contract_group_bound_auth.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/contract_group_bound_auth.rs @@ -1,6 +1,9 @@ use super::*; +use crate::execution::check_tx::CheckTxLevel; use crate::execution::validation::state_transition::tests::setup_identity_without_adding_it; +use crate::platform_types::platform::PlatformRef; use crate::rpc::core::MockCoreRPCLike; +use crate::test::helpers::contract_groups::{join_group, register_group, single_owner_info}; use crate::test::helpers::setup::TempPlatform; use dpp::consensus::codes::ErrorWithCode; use dpp::contract_group::{ @@ -11,6 +14,7 @@ use dpp::identity::contract_bounds::ContractBounds; use dpp::identity::identity_public_key::accessors::v0::IdentityPublicKeyGettersV0; use dpp::identity::{accessors::IdentitySettersV0, IdentityPublicKey}; use dpp::state_transition::batch_transition::methods::v1::DocumentsBatchTransitionMethodsV1; +use dpp::version::PLATFORM_VERSIONS; use std::collections::BTreeSet; const KEY_GROUP: [u8; 32] = [0x61; 32]; @@ -21,26 +25,12 @@ fn register_contract_group( contract_group_id: [u8; 32], version: &PlatformVersion, ) { - let info: ContractGroupInfo = ( - Identifier::from([0x60; 32]), - ContractGroupRegistration { - admins: BTreeSet::new(), - name: None, - description: None, - }, - ) - .into(); - platform - .drive - .insert_contract_group( - Identifier::from(contract_group_id), - &info, - &BlockInfo::default(), - true, - None, - version, - ) - .expect("expected to register the group"); + register_group( + platform, + Identifier::from(contract_group_id), + &single_owner_info(Identifier::from([0x60; 32]), None, None), + version, + ); } fn join_contract_group( @@ -50,20 +40,12 @@ fn join_contract_group( member: ContractGroupMember, version: &PlatformVersion, ) { - platform - .drive - .insert_contract_group_memberships( - contract_id, - &[ContractGroupMembership { - contract_group_id: Identifier::from(contract_group_id), - member, - }], - &BlockInfo::default(), - true, - None, - version, - ) - .expect("expected to record the membership"); + join_group( + platform, + contract_id, + &[(Identifier::from(contract_group_id), member)], + version, + ); } /// Sign with the original unbounded key metadata to bypass the SDK preflight; validators must @@ -167,6 +149,31 @@ async fn should_authorize_documents_by_the_contract_group_memberships_of_their_c .await .unwrap(); let bytes = batch.serialize_to_bytes().unwrap(); + + // CheckTx resolves the memberships through the same transformer and reaches the same + // verdict, so the mempool neither admits a batch outside the group nor drops one + // inside it. + let platform_ref = PlatformRef { + drive: &platform.drive, + state: &state, + config: &platform.config, + core_rpc: &platform.core_rpc, + }; + let check_tx_result = platform + .check_tx(&bytes, CheckTxLevel::FirstTimeCheck, &platform_ref, version) + .expect("expected to check tx"); + let check_tx_codes: Vec = check_tx_result + .errors + .iter() + .map(ErrorWithCode::code) + .collect(); + match case { + "whole_contract" | "document_type" | "joined_after_registration" => { + assert!(check_tx_codes.is_empty(), "{case}: {check_tx_codes:?}") + } + _ => assert_eq!(check_tx_codes, vec![20014], "{case}"), + } + let tx = platform.drive.grove.start_transaction(); let block_info = BlockInfo { time_ms: 100, @@ -432,7 +439,8 @@ async fn should_reject_non_batch_use_of_a_key_bound_to_a_contract_group() { /// The transformer reads a contract's group memberships only when the batch is signed by a key /// bound to a contract group. Any other batch is transformed exactly as before, with no extra -/// read, and a transformer that is not told who signed resolves nothing. +/// read, and a transformer that is not told who signed resolves nothing. Nor does it resolve +/// anything for a batch that already failed: such a result never reaches the bounds check. #[tokio::test] async fn should_resolve_contract_group_memberships_only_for_a_group_bound_signing_key() { use crate::execution::types::state_transition_execution_context::StateTransitionExecutionContext; @@ -444,7 +452,7 @@ async fn should_resolve_contract_group_memberships_only_for_a_group_bound_signin use dpp::version::DefaultForPlatformVersion; use drive::state_transition_action::StateTransitionAction; - for group_bound in [false, true] { + for (group_bound, fails_in_state) in [(false, false), (true, false), (true, true)] { let version = PlatformVersion::latest(); let platform = TestPlatformBuilder::new() .with_latest_protocol_version() @@ -500,20 +508,38 @@ async fn should_resolve_contract_group_memberships_only_for_a_group_bound_signin .unwrap(); set_valid_profile_payment_addresses(&mut document, profile); document.set("avatarUrl", "http://test.com/bob.jpg".into()); - let batch = BatchTransition::new_document_creation_transition_from_document( - document, - profile, - entropy.0, - &signing_key, - 2, - 0, - None, - &signer, - version, - None, - ) - .await - .unwrap(); + let batch = if fails_in_state { + // Replacing a profile that was never created fails in the transformer, which + // still yields an action (the nonce bump) next to the error. + BatchTransition::new_document_replacement_transition_from_document( + document, + profile, + &signing_key, + 2, + 0, + None, + &signer, + version, + None, + ) + .await + .unwrap() + } else { + BatchTransition::new_document_creation_transition_from_document( + document, + profile, + entropy.0, + &signing_key, + 2, + 0, + None, + &signer, + version, + None, + ) + .await + .unwrap() + }; let state = platform.state.load(); let platform_ref = PlatformRef { @@ -546,9 +572,13 @@ async fn should_resolve_contract_group_memberships_only_for_a_group_bound_signin ), } .expect("expected to transform the batch"); - let StateTransitionAction::BatchAction(action) = - result.into_data().expect("expected an action") - else { + assert_eq!( + result.errors.is_empty(), + !fails_in_state, + "{:?}", + result.errors + ); + let Some(StateTransitionAction::BatchAction(action)) = result.data else { panic!("expected a batch action"); }; action @@ -559,8 +589,8 @@ async fn should_resolve_contract_group_memberships_only_for_a_group_bound_signin let with_signer = resolved_for(Some(&signer_identity)); assert_eq!( with_signer.is_some(), - group_bound, - "group_bound={group_bound}" + group_bound && !fails_in_state, + "group_bound={group_bound} fails_in_state={fails_in_state}" ); if let Some(memberships) = with_signer { assert!(memberships.contains(&ContractGroupMembership { @@ -574,3 +604,26 @@ async fn should_resolve_contract_group_memberships_only_for_a_group_bound_signin ); } } + +/// Advanced structure v1 judges a group-bound key from the memberships that transform v2 +/// resolved into the action. Paired with an older transformer it would find none and fail every +/// such batch with a corrupted code execution error, so no protocol version may select one +/// without the other. +#[test] +fn should_pair_advanced_structure_v1_with_the_transformer_that_resolves_memberships() { + for version in PLATFORM_VERSIONS { + let batch = &version + .drive_abci + .validation_and_processing + .state_transitions + .batch_state_transition; + assert_eq!( + batch.advanced_structure >= 1, + batch.transform_into_action >= 2, + "protocol version {}: advanced structure {} with transform_into_action {}", + version.protocol_version, + batch.advanced_structure, + batch.transform_into_action + ); + } +} diff --git a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_update/mod.rs b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_update/mod.rs index e26bf7a22e2..bf26044f15d 100644 --- a/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_update/mod.rs +++ b/packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_update/mod.rs @@ -132,9 +132,11 @@ mod tests { setup_add_key_to_identity, setup_identity_return_master_key, }; use crate::platform_types::state_transitions_processing_result::StateTransitionExecutionResult; + use crate::test::helpers::contract_groups::{register_group, single_owner_info}; use crate::test::helpers::setup::TestPlatformBuilder; use assert_matches::assert_matches; use dpp::block::block_info::BlockInfo; + use dpp::consensus::codes::ErrorWithCode; use dpp::consensus::ConsensusError; use dpp::dash_to_credits; use dpp::dashcore::key::{Keypair, Secp256k1}; @@ -714,6 +716,200 @@ mod tests { .is_empty()); } + /// A key bound to a contract group is registered through the whole pipeline only once the + /// group exists; it is then indexed under the group, and stays bound after revocation. + #[tokio::test] + async fn should_register_and_revoke_an_authentication_key_bound_to_a_contract_group() { + use drive::drive::identity::key::fetch::IdentityKeysRequest; + let platform_version = PlatformVersion::latest(); + let mut platform = TestPlatformBuilder::new() + .with_latest_protocol_version() + .build_with_mock_rpc() + .set_genesis_state(); + let (identity, signer, _, key) = + setup_identity_return_master_key(&mut platform, 958, dash_to_credits!(0.1)); + let contract_group_id = Identifier::from([0x61; 32]); + let bounds = ContractBounds::ContractGroup { + id: contract_group_id, + }; + let platform_state = platform.state.load(); + let secp = Secp256k1::new(); + let mut rng = StdRng::seed_from_u64(293); + let new_key_pair = Keypair::new(&secp, &mut rng); + let build = + |revision: u64, nonce: u64, add: Vec, disable| { + StateTransition::from(IdentityUpdateTransition::from(IdentityUpdateTransitionV0 { + identity_id: identity.id(), + revision, + nonce, + add_public_keys: add + .into_iter() + .map(IdentityPublicKeyInCreation::V0) + .collect(), + disable_public_keys: disable, + user_fee_increase: 0, + signature_public_key_id: key.id(), + signature: Default::default(), + })) + }; + let mut signed_updates = Vec::new(); + for (revision, nonce) in [(1, 1), (1, 2)] { + let mut new_key = IdentityPublicKeyInCreationV0 { + id: 2, + purpose: Purpose::AUTHENTICATION, + security_level: SecurityLevel::HIGH, + key_type: ECDSA_SECP256K1, + read_only: false, + data: new_key_pair.public_key().serialize().to_vec().into(), + signature: Default::default(), + contract_bounds: Some(bounds.clone()), + }; + let signable_bytes = build(revision, nonce, vec![new_key.clone()], vec![]) + .signable_bytes() + .unwrap(); + new_key.signature = + signer::sign(&signable_bytes, &new_key_pair.secret_key().secret_bytes()) + .unwrap() + .to_vec() + .into(); + let mut update = build(revision, nonce, vec![new_key], vec![]); + update.set_signature(signer.sign(&key, signable_bytes.as_slice()).await.unwrap()); + signed_updates.push(update); + } + + // The group does not exist yet: a paid failure that registers nothing. + let transaction = platform.drive.grove.start_transaction(); + let result = platform + .platform + .process_raw_state_transitions( + &vec![signed_updates[0].serialize_to_bytes().unwrap()], + &platform_state, + &BlockInfo::default(), + &transaction, + platform_version, + true, + None, + ) + .unwrap(); + assert_matches!( + result.execution_results().as_slice(), + [StateTransitionExecutionResult::PaidConsensusError { error, .. }] + if error.code() == 41001 + ); + platform + .drive + .grove + .commit_transaction(transaction) + .unwrap() + .unwrap(); + + register_group( + &platform, + contract_group_id, + &single_owner_info(Identifier::from([0x60; 32]), None, None), + platform_version, + ); + let transaction = platform.drive.grove.start_transaction(); + let result = platform + .platform + .process_raw_state_transitions( + &vec![signed_updates[1].serialize_to_bytes().unwrap()], + &platform_state, + &BlockInfo::default(), + &transaction, + platform_version, + true, + None, + ) + .unwrap(); + assert_matches!( + result.execution_results().as_slice(), + [StateTransitionExecutionResult::SuccessfulExecution { .. }] + ); + platform + .drive + .grove + .commit_transaction(transaction) + .unwrap() + .unwrap(); + + // The key is indexed as the current authentication key of the group. + let group_keys_request = || { + IdentityKeysRequest::new_contract_group_authentication_keys_query( + identity.id().to_buffer(), + contract_group_id.to_buffer(), + ) + }; + let indexed = platform + .drive + .fetch_identity_keys_as_partial_identity(group_keys_request(), None, platform_version) + .unwrap() + .unwrap(); + assert_eq!( + indexed + .loaded_public_keys + .get(&2) + .unwrap() + .contract_bounds(), + Some(&bounds) + ); + + // Revocation through the master key refreshes the group references and keeps the + // bounds on the disabled key. + let mut revoke = build(2, 3, vec![], vec![2]); + revoke.set_signature( + signer + .sign(&key, &revoke.signable_bytes().unwrap()) + .await + .unwrap(), + ); + let block = BlockInfo { + time_ms: 50, + ..Default::default() + }; + let transaction = platform.drive.grove.start_transaction(); + let result = platform + .platform + .process_raw_state_transitions( + &vec![revoke.serialize_to_bytes().unwrap()], + &platform_state, + &block, + &transaction, + platform_version, + true, + None, + ) + .unwrap(); + assert_matches!( + result.execution_results().as_slice(), + [StateTransitionExecutionResult::SuccessfulExecution { .. }] + ); + platform + .drive + .grove + .commit_transaction(transaction) + .unwrap() + .unwrap(); + let fetched = platform + .drive + .fetch_identity_keys_as_partial_identity( + IdentityKeysRequest::new_specific_key_query(&identity.id().to_buffer(), 2), + None, + platform_version, + ) + .unwrap() + .unwrap(); + let revoked = fetched.loaded_public_keys.get(&2).unwrap(); + assert_eq!(revoked.disabled_at(), Some(50)); + assert_eq!(revoked.contract_bounds(), Some(&bounds)); + assert!(platform + .drive + .grove + .visualize_verify_grovedb(None, true, false, &platform_version.drive.grove_version) + .unwrap() + .is_empty()); + } + #[tokio::test] async fn should_refresh_every_bound_key_reference_after_revocation() { use drive::drive::identity::key::fetch::{ diff --git a/packages/rs-drive-abci/src/query/contract_group_queries/contract_group_info/v0/mod.rs b/packages/rs-drive-abci/src/query/contract_group_queries/contract_group_info/v0/mod.rs index 5ecf841d8f3..d00ddfefed2 100644 --- a/packages/rs-drive-abci/src/query/contract_group_queries/contract_group_info/v0/mod.rs +++ b/packages/rs-drive-abci/src/query/contract_group_queries/contract_group_info/v0/mod.rs @@ -86,10 +86,10 @@ impl Platform { mod tests { use super::*; use crate::error::query::QueryError; - use crate::query::contract_group_queries::tests::{ + use crate::query::tests::setup_platform; + use crate::test::helpers::contract_groups::{ owner_and_admins_info, register_group, single_owner_info, }; - use crate::query::tests::setup_platform; use dpp::dashcore::Network; use dpp::identifier::Identifier; use drive::drive::Drive; diff --git a/packages/rs-drive-abci/src/query/contract_group_queries/contract_group_members/v0/mod.rs b/packages/rs-drive-abci/src/query/contract_group_queries/contract_group_members/v0/mod.rs index 324971f0e4d..be9658d8b88 100644 --- a/packages/rs-drive-abci/src/query/contract_group_queries/contract_group_members/v0/mod.rs +++ b/packages/rs-drive-abci/src/query/contract_group_queries/contract_group_members/v0/mod.rs @@ -198,10 +198,8 @@ impl Platform { #[cfg(test)] mod tests { use super::*; - use crate::query::contract_group_queries::tests::{ - join_group, register_group, single_owner_info, - }; use crate::query::tests::setup_platform; + use crate::test::helpers::contract_groups::{join_group, register_group, single_owner_info}; use dapi_grpc::platform::v0::get_contract_group_members_request::{ ContractMembersQuery, DocumentTypeMembersQuery, TokenMembersQuery, }; diff --git a/packages/rs-drive-abci/src/query/contract_group_queries/contract_groups_for_contract/v0/mod.rs b/packages/rs-drive-abci/src/query/contract_group_queries/contract_groups_for_contract/v0/mod.rs index 588d19da239..f92623a58c8 100644 --- a/packages/rs-drive-abci/src/query/contract_group_queries/contract_groups_for_contract/v0/mod.rs +++ b/packages/rs-drive-abci/src/query/contract_group_queries/contract_groups_for_contract/v0/mod.rs @@ -100,10 +100,8 @@ impl Platform { mod tests { use super::*; use crate::error::query::QueryError; - use crate::query::contract_group_queries::tests::{ - join_group, register_group, single_owner_info, - }; use crate::query::tests::setup_platform; + use crate::test::helpers::contract_groups::{join_group, register_group, single_owner_info}; use dpp::contract_group::ContractGroupMember; use dpp::dashcore::Network; use dpp::identifier::Identifier; diff --git a/packages/rs-drive-abci/src/query/contract_group_queries/mod.rs b/packages/rs-drive-abci/src/query/contract_group_queries/mod.rs index 4adc8378113..44f77c4e243 100644 --- a/packages/rs-drive-abci/src/query/contract_group_queries/mod.rs +++ b/packages/rs-drive-abci/src/query/contract_group_queries/mod.rs @@ -19,94 +19,3 @@ pub(super) fn identifier_from_request( )) }) } - -#[cfg(test)] -pub(crate) mod tests { - use crate::rpc::core::MockCoreRPCLike; - use crate::test::helpers::setup::TempPlatform; - use dpp::block::block_info::BlockInfo; - use dpp::contract_group::{ - ContractGroupInfo, ContractGroupInfoV0, ContractGroupMember, ContractGroupMembership, - ContractGroupOwner, - }; - use dpp::identifier::Identifier; - use dpp::version::PlatformVersion; - use std::collections::BTreeSet; - - /// The information of a group with one owner. - pub fn single_owner_info( - owner: Identifier, - name: Option<&str>, - description: Option<&str>, - ) -> ContractGroupInfo { - ContractGroupInfo::V0(ContractGroupInfoV0 { - owner: ContractGroupOwner::SingleOwner(owner), - name: name.map(str::to_string), - description: description.map(str::to_string), - }) - } - - /// The information of a group with an owner and admins. - pub fn owner_and_admins_info( - owner: Identifier, - admins: &[Identifier], - name: Option<&str>, - description: Option<&str>, - ) -> ContractGroupInfo { - ContractGroupInfo::V0(ContractGroupInfoV0 { - owner: ContractGroupOwner::OwnerAndAdmins { - owner, - admins: admins.iter().copied().collect::>(), - }, - name: name.map(str::to_string), - description: description.map(str::to_string), - }) - } - - /// Registers a group straight into Drive's state. - pub fn register_group( - platform: &TempPlatform, - contract_group_id: Identifier, - info: &ContractGroupInfo, - platform_version: &PlatformVersion, - ) { - platform - .drive - .insert_contract_group( - contract_group_id, - info, - &BlockInfo::default(), - true, - None, - platform_version, - ) - .expect("expected to register the group"); - } - - /// Records a new contract's memberships straight into Drive's state. - pub fn join_group( - platform: &TempPlatform, - contract_id: Identifier, - memberships: &[(Identifier, ContractGroupMember)], - platform_version: &PlatformVersion, - ) { - let memberships: Vec = memberships - .iter() - .map(|(contract_group_id, member)| ContractGroupMembership { - contract_group_id: *contract_group_id, - member: member.clone(), - }) - .collect(); - platform - .drive - .insert_contract_group_memberships( - contract_id, - &memberships, - &BlockInfo::default(), - true, - None, - platform_version, - ) - .expect("expected to record the memberships"); - } -} diff --git a/packages/rs-drive-abci/src/test/helpers/contract_groups.rs b/packages/rs-drive-abci/src/test/helpers/contract_groups.rs new file mode 100644 index 00000000000..b726a98b961 --- /dev/null +++ b/packages/rs-drive-abci/src/test/helpers/contract_groups.rs @@ -0,0 +1,89 @@ +//! Contract group fixtures shared by the query tests and the validation tests. + +use crate::rpc::core::MockCoreRPCLike; +use crate::test::helpers::setup::TempPlatform; +use dpp::block::block_info::BlockInfo; +use dpp::contract_group::{ + ContractGroupInfo, ContractGroupInfoV0, ContractGroupMember, ContractGroupMembership, + ContractGroupOwner, +}; +use dpp::identifier::Identifier; +use dpp::version::PlatformVersion; +use std::collections::BTreeSet; + +/// The information of a group with one owner. +pub fn single_owner_info( + owner: Identifier, + name: Option<&str>, + description: Option<&str>, +) -> ContractGroupInfo { + ContractGroupInfo::V0(ContractGroupInfoV0 { + owner: ContractGroupOwner::SingleOwner(owner), + name: name.map(str::to_string), + description: description.map(str::to_string), + }) +} + +/// The information of a group with an owner and admins. +pub fn owner_and_admins_info( + owner: Identifier, + admins: &[Identifier], + name: Option<&str>, + description: Option<&str>, +) -> ContractGroupInfo { + ContractGroupInfo::V0(ContractGroupInfoV0 { + owner: ContractGroupOwner::OwnerAndAdmins { + owner, + admins: admins.iter().copied().collect::>(), + }, + name: name.map(str::to_string), + description: description.map(str::to_string), + }) +} + +/// Registers a group straight into Drive's state. +pub fn register_group( + platform: &TempPlatform, + contract_group_id: Identifier, + info: &ContractGroupInfo, + platform_version: &PlatformVersion, +) { + platform + .drive + .insert_contract_group( + contract_group_id, + info, + &BlockInfo::default(), + true, + None, + platform_version, + ) + .expect("expected to register the group"); +} + +/// Records a new contract's memberships straight into Drive's state. +pub fn join_group( + platform: &TempPlatform, + contract_id: Identifier, + memberships: &[(Identifier, ContractGroupMember)], + platform_version: &PlatformVersion, +) { + let memberships: Vec = memberships + .iter() + .map(|(contract_group_id, member)| ContractGroupMembership { + contract_group_id: *contract_group_id, + member: member.clone(), + }) + .collect(); + platform + .drive + .insert_contract_group_memberships( + contract_id, + &memberships, + &BlockInfo::default(), + true, + None, + platform_version, + ) + .expect("expected to record the memberships"); +} diff --git a/packages/rs-drive-abci/src/test/helpers/mod.rs b/packages/rs-drive-abci/src/test/helpers/mod.rs index 1a104a14a09..5c710ca8b4c 100644 --- a/packages/rs-drive-abci/src/test/helpers/mod.rs +++ b/packages/rs-drive-abci/src/test/helpers/mod.rs @@ -1,3 +1,6 @@ +/// Contract group fixtures +#[cfg(test)] +pub mod contract_groups; /// Test helpers #[cfg(test)] pub mod fast_forward_to_block; diff --git a/packages/rs-drive/src/drive/identity/contract_info/keys/mod.rs b/packages/rs-drive/src/drive/identity/contract_info/keys/mod.rs index 029b520c4ca..22ca9b60890 100644 --- a/packages/rs-drive/src/drive/identity/contract_info/keys/mod.rs +++ b/packages/rs-drive/src/drive/identity/contract_info/keys/mod.rs @@ -228,23 +228,14 @@ impl IdentityDataContractKeyApplyInfo { id: contract_group_id, } => { // Only authentication keys may be bound to a group (consensus refuses the - // rest); the group must exist, and the read is billed. + // rest). Nothing of the group is needed to build the apply info, so it is not + // read here: `fetch_bound_root_with_fee` checks that the group exists and + // bills that one read when the references are written. if purpose != Purpose::AUTHENTICATION { return Err(Error::Identity(IdentityError::IdentityKeyBoundsError( "only authentication keys can be bound to a contract group", ))); } - let info = drive.fetch_contract_group_info_add_to_operations( - *contract_group_id, - transaction, - drive_operations, - platform_version, - )?; - if info.is_none() { - return Err(Error::Identity(IdentityError::IdentityKeyBoundsError( - "Contract group for key bounds not found", - ))); - } return Ok(ContractGroupBased { contract_group_id: *contract_group_id, keys: vec![(key_id, purpose)], @@ -299,8 +290,35 @@ mod tests { identity_contract_info_group_keys_path_vec, identity_contract_info_group_path_key_purpose_vec, }; + use crate::util::test_helpers::setup::setup_drive_with_initial_state_structure; use grovedb::reference_path::ReferencePathType::SiblingReference; + /// The group of a group bound is read, checked and billed once, by + /// `fetch_bound_root_with_fee` when the references are written. Building the apply info + /// must not read it a second time. + #[test] + fn should_not_read_the_contract_group_when_building_the_apply_info() { + let platform_version = PlatformVersion::latest(); + let drive = setup_drive_with_initial_state_structure(None); + let contract_group_id = Identifier::from([0x61; 32]); + let mut drive_operations = vec![]; + let apply_info = IdentityDataContractKeyApplyInfo::new_from_single_key( + 7, + Purpose::AUTHENTICATION, + &ContractBounds::ContractGroup { + id: contract_group_id, + }, + &drive, + &Epoch::new(0).expect("expected epoch 0"), + None, + &mut drive_operations, + platform_version, + ) + .expect("expected the apply info of a group bound"); + assert!(drive_operations.is_empty(), "{drive_operations:?}"); + assert_eq!(apply_info.root_id(), contract_group_id.to_buffer()); + } + fn alias_insert(path: Vec>, key_id: KeyID) -> LowLevelDriveOperation { LowLevelDriveOperation::insert_for_known_path_key_element( path, diff --git a/packages/rs-platform-wallet-ffi/src/identity_persistence.rs b/packages/rs-platform-wallet-ffi/src/identity_persistence.rs index c9bf12756dc..90de62284b2 100644 --- a/packages/rs-platform-wallet-ffi/src/identity_persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/identity_persistence.rs @@ -1246,7 +1246,7 @@ mod tests { } #[test] - fn test_identity_key_entry_ffi_contract_bounds_contract_group() { + fn should_flatten_contract_group_bounds_as_kind_3() { use dpp::identity::identity_public_key::contract_bounds::ContractBounds; let contract_group_id = Identifier::from([0x47; 32]); let public_key = IdentityPublicKey::V0(IdentityPublicKeyV0 { diff --git a/packages/rs-platform-wallet-ffi/src/identity_registration_with_signer.rs b/packages/rs-platform-wallet-ffi/src/identity_registration_with_signer.rs index 49056d4bb31..22e301b5513 100644 --- a/packages/rs-platform-wallet-ffi/src/identity_registration_with_signer.rs +++ b/packages/rs-platform-wallet-ffi/src/identity_registration_with_signer.rs @@ -928,7 +928,7 @@ mod tests { } #[test] - fn decode_contract_bounds_kind_3_decodes_a_contract_group_and_rejects_null_id() { + fn should_decode_kind_3_as_a_contract_group_and_reject_a_null_id() { let pk = [0x02u8; 33]; let contract_group_id = [0x47u8; 32]; let mut row = ffi_row(0, &pk); diff --git a/packages/rs-unified-sdk-jni/src/pubkey_rows.rs b/packages/rs-unified-sdk-jni/src/pubkey_rows.rs index 89951f21187..f3f6dfcb7d8 100644 --- a/packages/rs-unified-sdk-jni/src/pubkey_rows.rs +++ b/packages/rs-unified-sdk-jni/src/pubkey_rows.rs @@ -548,7 +548,7 @@ mod tests { } #[test] - fn round_trips_contract_group_bounds_kind() { + fn should_round_trip_the_contract_group_bounds_kind() { let contract_group_id = [0x47u8; 32]; let rows = vec![Row { key_id: 2,