Skip to content

Commit

Permalink
Make changes to account owner keys a reserved instruction
Browse files Browse the repository at this point in the history
  • Loading branch information
0xOmarA committed Dec 13, 2024
1 parent 6159963 commit c1d95a7
Show file tree
Hide file tree
Showing 4 changed files with 163 additions and 30 deletions.
14 changes: 14 additions & 0 deletions crates/radix-engine-toolkit-uniffi/src/transaction_v1/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,13 +360,21 @@ impl ToNative for AccountDefaultDepositRule {
pub enum ReservedInstruction {
AccountLockFee,
AccountSecurify,
AccountUpdateOwnerKeysMetadataField,
IdentitySecurify,
IdentityUpdateOwnerKeysMetadataField,
AccessControllerMethod,
}

impl From<ReservedInstruction> for CoreReservedInstruction {
fn from(value: ReservedInstruction) -> Self {
match value {
ReservedInstruction::AccountUpdateOwnerKeysMetadataField => {
Self::AccountUpdateOwnerKeysMetadataField
}
ReservedInstruction::IdentityUpdateOwnerKeysMetadataField => {
Self::IdentityUpdateOwnerKeysMetadataField
}
ReservedInstruction::AccessControllerMethod => {
Self::AccessControllerMethod
}
Expand All @@ -380,6 +388,12 @@ impl From<ReservedInstruction> for CoreReservedInstruction {
impl From<CoreReservedInstruction> for ReservedInstruction {
fn from(value: CoreReservedInstruction) -> Self {
match value {
CoreReservedInstruction::AccountUpdateOwnerKeysMetadataField => {
Self::AccountUpdateOwnerKeysMetadataField
}
CoreReservedInstruction::IdentityUpdateOwnerKeysMetadataField => {
Self::IdentityUpdateOwnerKeysMetadataField
}
CoreReservedInstruction::AccessControllerMethod => {
Self::AccessControllerMethod
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
_ => {}
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions crates/radix-engine-toolkit/src/transaction_types/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,9 @@ pub struct NewEntities {
pub enum ReservedInstruction {
AccountLockFee,
AccountSecurify,
AccountUpdateOwnerKeysMetadataField,
IdentitySecurify,
IdentityUpdateOwnerKeysMetadataField,
AccessControllerMethod,
}

Expand Down
87 changes: 87 additions & 0 deletions crates/radix-engine-toolkit/tests/transaction_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
}

0 comments on commit c1d95a7

Please sign in to comment.