Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a general subintent tx type & make owner key changes reserved #129

Merged
merged 5 commits into from
Dec 13, 2024
Merged

Add a general subintent tx type & make owner key changes reserved #129

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6159963
Add a general subintent transaction type
0xOmarA Dec 13, 2024
c1d95a7
Make changes to account owner keys a reserved instruction
0xOmarA Dec 13, 2024
88d4a3a
formatting
0xOmarA Dec 13, 2024
cabe7d6
Disallow yielding to children in general subintent
0xOmarA Dec 13, 2024
e44b019
resolve review comments
0xOmarA Dec 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 4 additions & 7 deletions crates/radix-engine-toolkit-uniffi/src/blueprints/metadata.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,10 @@ impl ToNative for MetadataInit {
None => None,
};

Ok((
key,
NativeKeyValueStoreInitEntry::<NativeMetadataValue> {
lock: value.lock,
value: metadata,
},
))
Ok((key, NativeKeyValueStoreInitEntry::<NativeMetadataValue> {
lock: value.lock,
value: metadata,
}))
})
.collect::<Result<
IndexMap<
Expand Down
2 changes: 1 addition & 1 deletion crates/radix-engine-toolkit-uniffi/src/builder/manifest_builder/builder_v1.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
#![allow(clippy::too_many_arguments)]

use crate::prelude::*;
use radix_common::prelude::{to_manifest_value, FromPublicKey};
use radix_common::prelude::{FromPublicKey, to_manifest_value};

#[derive(Debug, Clone, Object, Default)]
pub struct ManifestV1Builder {
Expand Down
2 changes: 1 addition & 1 deletion crates/radix-engine-toolkit-uniffi/src/builder/manifest_builder/builder_v2.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
#![allow(clippy::too_many_arguments)]

use crate::prelude::*;
use radix_common::prelude::{to_manifest_value, FromPublicKey};
use radix_common::prelude::{FromPublicKey, to_manifest_value};

#[derive(Debug, Clone, Object)]
pub struct ManifestV2Builder {
Expand Down
30 changes: 30 additions & 0 deletions crates/radix-engine-toolkit-uniffi/src/transaction_v1/manifest.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,13 +360,29 @@ impl ToNative for AccountDefaultDepositRule {
pub enum ReservedInstruction {
AccountLockFee,
AccountSecurify,
AccountLockOwnerKeysMetadataField,
AccountUpdateOwnerKeysMetadataField,
IdentitySecurify,
IdentityLockOwnerKeysMetadataField,
IdentityUpdateOwnerKeysMetadataField,
AccessControllerMethod,
}

impl From<ReservedInstruction> for CoreReservedInstruction {
fn from(value: ReservedInstruction) -> Self {
match value {
ReservedInstruction::AccountUpdateOwnerKeysMetadataField => {
Self::AccountUpdateOwnerKeysMetadataField
}
ReservedInstruction::AccountLockOwnerKeysMetadataField => {
Self::AccountLockOwnerKeysMetadataField
}
ReservedInstruction::IdentityUpdateOwnerKeysMetadataField => {
Self::IdentityUpdateOwnerKeysMetadataField
}
ReservedInstruction::IdentityLockOwnerKeysMetadataField => {
Self::IdentityLockOwnerKeysMetadataField
}
ReservedInstruction::AccessControllerMethod => {
Self::AccessControllerMethod
}
Expand All @@ -380,6 +396,18 @@ impl From<ReservedInstruction> for CoreReservedInstruction {
impl From<CoreReservedInstruction> for ReservedInstruction {
fn from(value: CoreReservedInstruction) -> Self {
match value {
CoreReservedInstruction::AccountUpdateOwnerKeysMetadataField => {
Self::AccountUpdateOwnerKeysMetadataField
}
CoreReservedInstruction::AccountLockOwnerKeysMetadataField => {
Self::AccountLockOwnerKeysMetadataField
}
CoreReservedInstruction::IdentityUpdateOwnerKeysMetadataField => {
Self::IdentityUpdateOwnerKeysMetadataField
}
CoreReservedInstruction::IdentityLockOwnerKeysMetadataField => {
Self::IdentityLockOwnerKeysMetadataField
}
CoreReservedInstruction::AccessControllerMethod => {
Self::AccessControllerMethod
}
Expand All @@ -392,6 +420,7 @@ impl From<CoreReservedInstruction> for ReservedInstruction {

#[derive(Clone, Debug, Enum)]
pub enum ManifestClass {
GeneralSubintent,
General,
Transfer,
PoolContribution,
Expand All @@ -405,6 +434,7 @@ pub enum ManifestClass {
impl From<CoreManifestClass> for ManifestClass {
fn from(value: CoreManifestClass) -> Self {
match value {
CoreManifestClass::GeneralSubintent => Self::GeneralSubintent,
CoreManifestClass::General => Self::General,
CoreManifestClass::Transfer => Self::Transfer,
CoreManifestClass::PoolContribution => Self::PoolContribution,
Expand Down
2 changes: 1 addition & 1 deletion crates/radix-engine-toolkit/src/functions/manifest_sbor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
// under the License.

use radix_transactions::data::{
format_manifest_value, ManifestDecompilationDisplayContext,
ManifestDecompilationDisplayContext, format_manifest_value,
};
use sbor::prelude::ContextualSerialize;
use sbor::representations::{SerializationMode, SerializationParameters};
Expand Down
2 changes: 1 addition & 1 deletion crates/radix-engine-toolkit/src/functions/transaction_v1/manifest.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
use radix_common::prelude::*;
use radix_engine_toolkit_common::receipt::RuntimeToolkitTransactionReceipt;
use radix_transactions::errors::*;
use radix_transactions::manifest::static_resource_movements::StaticResourceMovementsError;
use radix_transactions::manifest::BuildableManifest;
use radix_transactions::manifest::static_resource_movements::StaticResourceMovementsError;
use radix_transactions::prelude::*;
use radix_transactions::validation::*;

Expand Down
13 changes: 5 additions & 8 deletions crates/radix-engine-toolkit/src/models/canonical_address_types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,14 +295,11 @@ mod tests {
let input = "account_sim1cyvgx33089ukm2pl97pv4max0x40ruvfy4lt60yvya744cve475w0q";

let x = CanonicalAccountAddress::from_str(input).unwrap();
assert_eq!(
x.node_id.as_bytes(),
[
193, 24, 131, 70, 47, 57, 121, 109, 168, 63, 47, 130, 202, 239,
166, 121, 170, 241, 241, 137, 37, 126, 189, 60, 140, 39, 125,
90, 225, 153
]
);
assert_eq!(x.node_id.as_bytes(), [
193, 24, 131, 70, 47, 57, 121, 109, 168, 63, 47, 130, 202, 239,
166, 121, 170, 241, 241, 137, 37, 126, 189, 60, 140, 39, 125, 90,
225, 153
]);
assert_eq!(x.network_id, 0xf2);
assert_eq!(x.to_string(), input);

Expand Down
46 changes: 35 additions & 11 deletions crates/radix-engine-toolkit/src/transaction_types/interface.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
// specific language governing permissions and limitations
// under the License.

// TODO: Refactor the functions in here into a single function perhaps through
// some form of parsing modes, but we need to deduplicate the logic.

//! Functions that expose the transaction types functionality without exposing
//! any of the implementation details of how the module finds and determines
//! the transaction types.
Expand Down Expand Up @@ -44,6 +47,8 @@ pub fn statically_analyze<M: ReadableManifest + ?Sized>(
StaticAccountResourceMovementsDetector::default();

let mut general_transaction_detector = GeneralDetector::default();
let mut general_subintent_transaction_detector =
GeneralSubintentDetector::default();
let mut transfer_transaction_detector = TransferDetector::default();
let mut pool_contribution_detector = PoolContributionDetector::default();
let mut pool_redemption_detector = PoolRedemptionDetector::default();
Expand All @@ -62,6 +67,7 @@ pub fn statically_analyze<M: ReadableManifest + ?Sized>(
&mut reserved_instructions_detector,
&mut account_resource_movements_detector,
&mut general_transaction_detector,
&mut general_subintent_transaction_detector,
&mut transfer_transaction_detector,
&mut pool_contribution_detector,
&mut pool_redemption_detector,
Expand All @@ -82,6 +88,10 @@ pub fn statically_analyze<M: ReadableManifest + ?Sized>(
let (account_withdraws, account_deposits) =
account_resource_movements_detector.output();
let classification = [
(
ManifestClass::GeneralSubintent,
general_subintent_transaction_detector.is_valid(),
),
(
ManifestClass::General,
general_transaction_detector.is_valid(),
Expand Down Expand Up @@ -146,6 +156,8 @@ pub fn statically_analyze_and_validate<M: ReadableManifest + ?Sized>(
StaticAccountResourceMovementsDetector::default();

let mut general_transaction_detector = GeneralDetector::default();
let mut general_subintent_transaction_detector =
GeneralSubintentDetector::default();
let mut transfer_transaction_detector = TransferDetector::default();
let mut pool_contribution_detector = PoolContributionDetector::default();
let mut pool_redemption_detector = PoolRedemptionDetector::default();
Expand All @@ -164,6 +176,7 @@ pub fn statically_analyze_and_validate<M: ReadableManifest + ?Sized>(
&mut reserved_instructions_detector,
&mut account_resource_movements_detector,
&mut general_transaction_detector,
&mut general_subintent_transaction_detector,
&mut transfer_transaction_detector,
&mut pool_contribution_detector,
&mut pool_redemption_detector,
Expand All @@ -184,6 +197,10 @@ pub fn statically_analyze_and_validate<M: ReadableManifest + ?Sized>(
let (account_withdraws, account_deposits) =
account_resource_movements_detector.output();
let classification = [
(
ManifestClass::GeneralSubintent,
general_subintent_transaction_detector.is_valid(),
),
(
ManifestClass::General,
general_transaction_detector.is_valid(),
Expand Down Expand Up @@ -262,6 +279,8 @@ pub fn classify_manifest<M: ReadableManifest + ?Sized>(
StaticAccountResourceMovementsDetector::default();

let mut general_transaction_detector = GeneralDetector::default();
let mut general_subintent_transaction_detector =
GeneralSubintentDetector::default();
let mut transfer_transaction_detector = TransferDetector::default();
let mut pool_contribution_detector = PoolContributionDetector::default();
let mut pool_redemption_detector = PoolRedemptionDetector::default();
Expand All @@ -280,6 +299,7 @@ pub fn classify_manifest<M: ReadableManifest + ?Sized>(
&mut reserved_instructions_detector,
&mut account_resource_movements_detector,
&mut general_transaction_detector,
&mut general_subintent_transaction_detector,
&mut transfer_transaction_detector,
&mut pool_contribution_detector,
&mut pool_redemption_detector,
Expand All @@ -293,6 +313,10 @@ pub fn classify_manifest<M: ReadableManifest + ?Sized>(

// Extracting the data out of the detectors and into the ManifestSummary
[
(
ManifestClass::GeneralSubintent,
general_subintent_transaction_detector.is_valid(),
),
(
ManifestClass::General,
general_transaction_detector.is_valid(),
Expand Down Expand Up @@ -352,6 +376,8 @@ pub fn dynamically_analyze<M: ReadableManifest>(
let newly_created_non_fungibles = receipt.new_non_fungibles();

let mut general_transaction_detector = GeneralDetector::default();
let mut general_subintent_transaction_detector =
GeneralSubintentDetector::default();
let mut transfer_transaction_detector = TransferDetector::default();
let mut pool_contribution_detector = PoolContributionDetector::default();
let mut pool_redemption_detector = PoolRedemptionDetector::default();
Expand All @@ -370,6 +396,7 @@ pub fn dynamically_analyze<M: ReadableManifest>(
&mut reserved_instructions_detector,
&mut account_resource_movements_detector,
&mut general_transaction_detector,
&mut general_subintent_transaction_detector,
&mut transfer_transaction_detector,
&mut pool_contribution_detector,
&mut pool_redemption_detector,
Expand Down Expand Up @@ -486,17 +513,14 @@ pub fn dynamically_analyze<M: ReadableManifest>(
k,
v.into_iter()
.map(|(badge, operation)| {
(
badge,
match operation {
Update::Set(()) => {
Operation::Added
}
Update::Remove => {
Operation::Removed
}
},
)
(badge, match operation {
Update::Set(()) => {
Operation::Added
}
Update::Remove => {
Operation::Removed
}
})
})
.collect(),
)
Expand Down
108 changes: 78 additions & 30 deletions ...s/radix-engine-toolkit/src/transaction_types/traverser/auxiliary/reserved_instructions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,36 +39,84 @@ 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 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> { .. }

// 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 {
0xOmarA marked this conversation as resolved.
Show resolved Hide resolved
// 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);
}
} else if method_name == METADATA_LOCK_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(MetadataLockInput { 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::AccountLockOwnerKeysMetadataField);
} else if is_identity(address) && is_owner_keys_metadata_key
{
self.reserved_instructions.insert(ReservedInstruction::IdentityLockOwnerKeysMetadataField);
}
}
}
_ => {}
}
}
}
Expand Down
Loading
Loading