diff --git a/crates/radix-engine-toolkit-uniffi/src/transaction_v1/manifest.rs b/crates/radix-engine-toolkit-uniffi/src/transaction_v1/manifest.rs index eadd446c..e1b5e8dd 100644 --- a/crates/radix-engine-toolkit-uniffi/src/transaction_v1/manifest.rs +++ b/crates/radix-engine-toolkit-uniffi/src/transaction_v1/manifest.rs @@ -360,13 +360,21 @@ impl ToNative for AccountDefaultDepositRule { pub enum ReservedInstruction { AccountLockFee, AccountSecurify, + AccountUpdateOwnerKeysMetadataField, IdentitySecurify, + IdentityUpdateOwnerKeysMetadataField, AccessControllerMethod, } impl From for CoreReservedInstruction { fn from(value: ReservedInstruction) -> Self { match value { + ReservedInstruction::AccountUpdateOwnerKeysMetadataField => { + Self::AccountUpdateOwnerKeysMetadataField + } + ReservedInstruction::IdentityUpdateOwnerKeysMetadataField => { + Self::IdentityUpdateOwnerKeysMetadataField + } ReservedInstruction::AccessControllerMethod => { Self::AccessControllerMethod } @@ -380,6 +388,12 @@ impl From for CoreReservedInstruction { impl From for ReservedInstruction { fn from(value: CoreReservedInstruction) -> Self { match value { + CoreReservedInstruction::AccountUpdateOwnerKeysMetadataField => { + Self::AccountUpdateOwnerKeysMetadataField + } + CoreReservedInstruction::IdentityUpdateOwnerKeysMetadataField => { + Self::IdentityUpdateOwnerKeysMetadataField + } CoreReservedInstruction::AccessControllerMethod => { Self::AccessControllerMethod } diff --git a/crates/radix-engine-toolkit/src/transaction_types/traverser/auxiliary/reserved_instructions.rs b/crates/radix-engine-toolkit/src/transaction_types/traverser/auxiliary/reserved_instructions.rs index 8494b75d..96df6205 100644 --- a/crates/radix-engine-toolkit/src/transaction_types/traverser/auxiliary/reserved_instructions.rs +++ b/crates/radix-engine-toolkit/src/transaction_types/traverser/auxiliary/reserved_instructions.rs @@ -39,36 +39,66 @@ impl ReservedInstructionsDetector { impl StaticAnalysisCallback for ReservedInstructionsDetector { fn on_instruction(&mut self, instruction: &InstructionV2, _: usize) { - let InstructionV2::CallMethod(CallMethod { - address, - method_name, - .. - }) = instruction - else { - return; - }; - - if is_account(address) - && contains!( - method_name => [ - ACCOUNT_LOCK_FEE_IDENT, - ACCOUNT_LOCK_FEE_AND_WITHDRAW_IDENT, - ACCOUNT_LOCK_FEE_AND_WITHDRAW_NON_FUNGIBLES_IDENT, - ] - ) - { - self.reserved_instructions - .insert(ReservedInstruction::AccountLockFee); - } else if is_account(address) && method_name == ACCOUNT_SECURIFY_IDENT { - self.reserved_instructions - .insert(ReservedInstruction::AccountSecurify); - } else if is_identity(address) && method_name == IDENTITY_SECURIFY_IDENT - { - self.reserved_instructions - .insert(ReservedInstruction::AccountLockFee); - } else if is_access_controller(address) { - self.reserved_instructions - .insert(ReservedInstruction::AccessControllerMethod); + // TODO: Make use of the typed manifest invocations - they would make + // some of the logic in here easier. + match instruction { + InstructionV2::CallMethod(CallMethod { + address, + method_name, + .. + }) => { + if is_account(address) + && contains!( + method_name => [ + ACCOUNT_LOCK_FEE_IDENT, + ACCOUNT_LOCK_FEE_AND_WITHDRAW_IDENT, + ACCOUNT_LOCK_FEE_AND_WITHDRAW_NON_FUNGIBLES_IDENT, + ] + ) + { + self.reserved_instructions + .insert(ReservedInstruction::AccountLockFee); + } else if is_account(address) + && method_name == ACCOUNT_SECURIFY_IDENT + { + self.reserved_instructions + .insert(ReservedInstruction::AccountSecurify); + } else if is_identity(address) + && method_name == IDENTITY_SECURIFY_IDENT + { + self.reserved_instructions + .insert(ReservedInstruction::AccountLockFee); + } else if is_access_controller(address) { + self.reserved_instructions + .insert(ReservedInstruction::AccessControllerMethod); + } + } + InstructionV2::CallMetadataMethod(CallMetadataMethod { + address, + method_name, + args, + }) => { + if method_name == METADATA_SET_IDENT { + // Attempt to decode the args as a metadata set call. If we + // fail then we have not technically detected a violation of + // the reserved instructions and we can just ignore this. + let Some(MetadataSetInput { key, .. }) = + manifest_encode(args) + .ok() + .and_then(|encoded| manifest_decode(&encoded).ok()) + else { + return; + }; + let is_owner_keys_metadata_key = key == "owner_keys"; + if is_account(address) && is_owner_keys_metadata_key { + self.reserved_instructions.insert(ReservedInstruction::AccountUpdateOwnerKeysMetadataField); + } else if is_identity(address) && is_owner_keys_metadata_key + { + self.reserved_instructions.insert(ReservedInstruction::IdentityUpdateOwnerKeysMetadataField); + } + } + } + _ => {} } } } diff --git a/crates/radix-engine-toolkit/src/transaction_types/types.rs b/crates/radix-engine-toolkit/src/transaction_types/types.rs index 84f4eeba..1bf12214 100644 --- a/crates/radix-engine-toolkit/src/transaction_types/types.rs +++ b/crates/radix-engine-toolkit/src/transaction_types/types.rs @@ -486,7 +486,9 @@ pub struct NewEntities { pub enum ReservedInstruction { AccountLockFee, AccountSecurify, + AccountUpdateOwnerKeysMetadataField, IdentitySecurify, + IdentityUpdateOwnerKeysMetadataField, AccessControllerMethod, } diff --git a/crates/radix-engine-toolkit/tests/transaction_types.rs b/crates/radix-engine-toolkit/tests/transaction_types.rs index 738e7a37..62c7f380 100644 --- a/crates/radix-engine-toolkit/tests/transaction_types.rs +++ b/crates/radix-engine-toolkit/tests/transaction_types.rs @@ -2576,3 +2576,90 @@ fn subintent_manifest_classification_test( assert!(!contains_general_subintent_classification); } } + +#[test] +fn update_to_account_owner_keys_is_reserved_instruction() { + let account = + ComponentAddress::new_or_panic([EntityType::GlobalAccount as u8; 30]); + reserved_instructions_test( + ManifestBuilder::new() + .set_metadata(account, "owner_keys", "not relevant") + .build(), + true, + false, + ) +} + +#[test] +fn update_to_identity_owner_keys_is_reserved_instruction() { + let identity = + ComponentAddress::new_or_panic([EntityType::GlobalIdentity as u8; 30]); + reserved_instructions_test( + ManifestBuilder::new() + .set_metadata(identity, "owner_keys", "not relevant") + .build(), + false, + true, + ) +} + +#[test] +fn update_to_component_owner_keys_is_not_reserved_instruction() { + let component = ComponentAddress::new_or_panic( + [EntityType::GlobalGenericComponent as u8; 30], + ); + reserved_instructions_test( + ManifestBuilder::new() + .set_metadata(component, "owner_keys", "not relevant") + .build(), + false, + false, + ) +} + +#[test] +fn update_to_account_non_owner_keys_field_is_not_reserved_instruction() { + let account = + ComponentAddress::new_or_panic([EntityType::GlobalAccount as u8; 30]); + reserved_instructions_test( + ManifestBuilder::new() + .set_metadata(account, "example", "not relevant") + .build(), + false, + false, + ) +} + +#[test] +fn update_to_identity_non_owner_keys_field_is_not_reserved_instruction() { + let identity = + ComponentAddress::new_or_panic([EntityType::GlobalIdentity as u8; 30]); + reserved_instructions_test( + ManifestBuilder::new() + .set_metadata(identity, "example", "not relevant") + .build(), + false, + false, + ) +} + +fn reserved_instructions_test( + manifest: TransactionManifestV1, + expect_account_owner_key_update: bool, + expect_identity_owner_key_update: bool, +) { + let StaticAnalysis { + reserved_instructions, + .. + } = statically_analyze(&manifest); + let [is_account_owner_key_update, is_identity_owner_key_update] = [ + ReservedInstruction::AccountUpdateOwnerKeysMetadataField, + ReservedInstruction::IdentityUpdateOwnerKeysMetadataField, + ] + .map(|expected| reserved_instructions.contains(&expected)); + assert_eq!(is_account_owner_key_update, expect_account_owner_key_update); + assert_eq!( + is_identity_owner_key_update, + expect_identity_owner_key_update + ); +}