Add a general subintent tx type & make owner key changes reserved #129
Conversation
crates/radix-engine-toolkit/src/transaction_types/traverser/types/general_subintent.rs
Outdated
Show resolved
Hide resolved
crates/radix-engine-toolkit/src/transaction_types/traverser/types/general_subintent.rs
Outdated
Show resolved
Hide resolved
crates/radix-engine-toolkit/src/transaction_types/traverser/types/general_subintent.rs
Show resolved
Hide resolved
crates/radix-engine-toolkit/src/transaction_types/traverser/types/general_subintent.rs
Outdated
Show resolved
Hide resolved
| } | ||
| } | ||
|
|
||
| fn on_instruction(&mut self, instruction: &InstructionV2, _: usize) { |
There was a problem hiding this comment.
VERY MINOR / FOR FUTURE: In general, we can use AnyInstruction here instead of InstructionV2 (it's just an alias for the latest version, which would mean less code needs changing here if we add new instructions in future).
crates/radix-engine-toolkit/src/transaction_types/traverser/auxiliary/reserved_instructions.rs
Show resolved
Hide resolved
| } else if is_access_controller(address) { | ||
| self.reserved_instructions | ||
| .insert(ReservedInstruction::AccessControllerMethod); | ||
| // TODO: Make use of the typed manifest invocations - they would make |
There was a problem hiding this comment.
👍 Sounds good.
VERY MINOR/OPTIONAL: This logic would probably be a little bit safer as if let Some(reserved) = as_reserved(instruction) { /* add to list */ } for a fn is_reserved(instruction &AnyInstruction) -> Option<ReservedInstruction> { .. }
| | InstructionV2::VerifyParent(_) => true, | ||
| /* Not Permitted */ | ||
| InstructionV2::BurnResource(..) | ||
| | InstructionV2::CallRoyaltyMethod(..) | ||
| | InstructionV2::CallMetadataMethod(..) | ||
| | InstructionV2::CallRoleAssignmentMethod(..) | ||
| | InstructionV2::CallDirectVaultMethod(..) | ||
| | InstructionV2::AllocateGlobalAddress(..) => false, | ||
| | InstructionV2::AllocateGlobalAddress(..) | ||
| | InstructionV2::YieldToChild(_) => false, |
There was a problem hiding this comment.
Can we add a test for this? i.e. that a subintent yielding to CHILD is not matched?
There was a problem hiding this comment.
Similarly, given it's a requirement for the Gateway to return the correct classification for the MVP, can we have a test where a simple transaction intent manifest which has a YIELD_TO_CHILD is explicitly unclassified? (with a comment that this behaviour is needed for the wallet to mark transactions-with-subintents as Complex; and if changed, we should ensure we change it in line with Gateway and/or wallet behaviour)
No description provided.