Skip to content

Commit

Permalink
Merge pull request #129 from radixdlt/bugfix/general-transaction-type
Browse files Browse the repository at this point in the history 
Add a general subintent tx type & make owner key changes reserved
  • Loading branch information
@0xOmarA
0xOmarA authored Dec 13, 2024
2 parents f985596 + e44b019 commit e350931
Show file tree
Hide file tree
Showing 20 changed files with 1,094 additions and 782 deletions.
11 changes: 4 additions & 7 deletions crates/radix-engine-toolkit-uniffi/src/blueprints/metadata.rs
View file
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
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
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
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
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
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
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
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
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
// 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);
}
} 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

0 comments on commit e350931

Please sign in to comment.